Linux-Wireless Archive on lore.kernel.org
 help / Atom feed
* [RFC PATCH v3 00/12] Draft for Extended Key ID support
@ 2019-02-10 21:06 Alexander Wetzel
  2019-02-10 21:06 ` [RFC PATCH v3 01/12] mac80211: Optimize tailroom_needed update checks Alexander Wetzel
                   ` (12 more replies)
  0 siblings, 13 replies; 23+ messages in thread
From: Alexander Wetzel @ 2019-02-10 21:06 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Alexander Wetzel

This is my current development version for Extended Key ID support in
linux and mac80211.
I consider the all patches in this series against nl80211/mac80211 ready
for merge and if they still have defects not mentioned in the patch I
need your help to see them.
There are still some questions if we even want/need all those patches,
and so I've added some remarks to behind some commit message to start the
different discussions.

The driver patches are - with the exception of the hwsim patch -
definitely not ready for merge and mostly here to illustrate how the
different APIs can be used and to start some discussions how to handle HW
specific challenges. Of course if someone wants to play with Extended Key
ID they also should be useful... (I can provide updated mostly working
hostapd/wpa_supplicant patches if someone is interested. Don't try to
use the old ones I sent to hostapd mailing list in November.)

That said I'm now using most of the patches or their predecessor in my
private Wlan with devices both supporting and not supporting Extended
Key ID fine.

Compared to the last RFC patch only the nl80211 patch is still close to
what we discussed. It got the API cleanup/changes and the open sanity
checks and not much more.

The mac80211 patch from RFC v2 had serious defects. The most serious one
was probably to not select the key based on the keyid of the MPDU.
I think outlining all the changes will not be useful here, the initial
patch was too broken for anything but SW crypto. (Which also had
issues...)
It started out with more or less all the fixes we discussed but when
trying to get it really correct and feature complete it became three
different patches we better review from the scratch. They are now
touching much more code and make in some cases drastic changes.

Here a short overview of the patches in the series and why they are in
it:

1) mac80211: Optimize tailroom_needed update checks:
   This would be a standalone patch, but some other patches depend on it
   to apply cleanly.

2) nl80211/cfg80211: Extended Key ID support
   Generic support for Extended Key ID.

3) mac80211: IEEE 802.11 Extended Key ID support
   Mac80211 Extended Key ID support for drivers when the hardware is able to
   handle Extended Key ID (aka two pairwise keys in HW).

4) mac80211: Compatibility Extended Key ID support
   Mac80211 Extended Key ID support for most devices not able to handle
   two unicast keys in HW.

5) mac80211: Mark A-MPDU keyid borders for drivers
   This is one big question, see the patch for why we may want this or
   not...

6) mac80211_hwsim: Ext Key ID support (NATIVE)
   Just a one-liner to allow Extended Key ID to be used with hwsim.

--- No patch below this line is ready for merge ---

7) iwlwifi: Extended Key ID support (NATIVE)
   Hopefully the seed to support Extended Key ID for all iwlwifi cards,
   see the patch description for the (big) issue it has.
   As it is it's mostly an example how Native Extended Key ID support
   will look like working with only some cards.

8) iwlwifi: dvm - EXT_KEY_ID A-MPDU API update
   Stops iwldvm drivers to complain when used together with the
   experimental "mac80211: Mark A-MPDU keyid boarders for drivers"
   patch.

The following patches in the series are only illustrating the COMPAT
Extended Key ID support:

9) ath: Basic Extended Key ID support
   Experimental patch for generic Extended Key ID support for all ath
   drivers.

10) ath5k: ath_key_config() API compatibility update
    Allows to still compile ath5k drivers with the patch above.
    Only provided to not break any drivers if someone wants to test
    this.

11) ath9k: Extended Key ID support (COMPAT)
    The example for Compatibility Key ID support, works together with
    "ath: Basic Extended Key ID support".
 
12) ath9k: EXT_KEY_ID A-MPDU API update
    A mostly untested example how drivers may benefit from "mac80211:
    Mark A-MPDU keyid boarders for drivers".

Alexander Wetzel (12):
  mac80211: Optimize tailroom_needed update checks
  nl80211/cfg80211: Extended Key ID support
  mac80211: IEEE 802.11 Extended Key ID support
  mac80211: Compatibility Extended Key ID support
  mac80211: Mark A-MPDU keyid boarders for drivers
  mac80211_hwsim: Ext Key ID support (NATIVE)
  iwlwifi: Extended Key ID support (NATIVE)
  iwlwifi: dvm - EXT_KEY_ID A-MPDU API update
  ath: Basic Extended Key ID support (COMPAT+NATIVE)
  ath5k: ath_key_config() API compatibility update
  ath9k: Extended Key ID support (COMPAT)
  ath9k: EXT_KEY_ID A-MPDU API update

 drivers/net/wireless/ath/ath.h                |   7 +-
 drivers/net/wireless/ath/ath5k/mac80211-ops.c |   2 +-
 drivers/net/wireless/ath/ath9k/htc_drv_main.c |   2 +-
 drivers/net/wireless/ath/ath9k/init.c         |   1 +
 drivers/net/wireless/ath/ath9k/main.c         |  20 +-
 drivers/net/wireless/ath/ath9k/xmit.c         |  14 +-
 drivers/net/wireless/ath/key.c                |  35 ++-
 .../net/wireless/intel/iwlwifi/dvm/mac80211.c |   5 +
 drivers/net/wireless/intel/iwlwifi/dvm/tx.c   |   2 +-
 .../net/wireless/intel/iwlwifi/mvm/mac80211.c |   5 +
 drivers/net/wireless/mac80211_hwsim.c         |   1 +
 include/net/cfg80211.h                        |   2 +
 include/net/mac80211.h                        |  65 ++++-
 include/uapi/linux/nl80211.h                  |  23 +-
 net/mac80211/cfg.c                            |  38 +++
 net/mac80211/debugfs.c                        |   2 +
 net/mac80211/ieee80211_i.h                    |   2 +-
 net/mac80211/key.c                            | 223 +++++++++++++++---
 net/mac80211/key.h                            |   9 +
 net/mac80211/main.c                           |   6 +
 net/mac80211/rx.c                             |  81 ++++---
 net/mac80211/sta_info.c                       |  13 +
 net/mac80211/sta_info.h                       |   6 +-
 net/mac80211/tx.c                             |  77 ++++--
 net/wireless/nl80211.c                        |  32 ++-
 net/wireless/rdev-ops.h                       |   3 +-
 net/wireless/trace.h                          |  31 ++-
 net/wireless/util.c                           |  20 +-
 28 files changed, 601 insertions(+), 126 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [RFC PATCH v3 01/12] mac80211: Optimize tailroom_needed update checks
  2019-02-10 21:06 [RFC PATCH v3 00/12] Draft for Extended Key ID support Alexander Wetzel
@ 2019-02-10 21:06 ` Alexander Wetzel
  2019-02-10 21:06 ` [RFC PATCH v3 02/12] nl80211/cfg80211: Extended Key ID support Alexander Wetzel
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 23+ messages in thread
From: Alexander Wetzel @ 2019-02-10 21:06 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Alexander Wetzel

Optimize/cleanup the delay tailroom checks and adds one missing tailroom
update.

Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de>
---

It may make sense to write a small macro for support here.
This is only a small tweak/fix of the existing code to fit to how it's
handled in the Extended Key ID patches.

 net/mac80211/key.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/net/mac80211/key.c b/net/mac80211/key.c
index 37e372896230..b9f2bfc00263 100644
--- a/net/mac80211/key.c
+++ b/net/mac80211/key.c
@@ -140,6 +140,12 @@ static int ieee80211_key_enable_hw_accel(struct ieee80211_key *key)
 		 * so clear that flag now to avoid trying to remove
 		 * it again later.
 		 */
+		if (key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE &&
+		    !(key->conf.flags & (IEEE80211_KEY_FLAG_GENERATE_MMIC |
+					 IEEE80211_KEY_FLAG_PUT_MIC_SPACE |
+					 IEEE80211_KEY_FLAG_RESERVE_TAILROOM)))
+			increment_tailroom_need_count(sdata);
+
 		key->flags &= ~KEY_FLAG_UPLOADED_TO_HARDWARE;
 		return -EINVAL;
 	}
@@ -179,9 +185,9 @@ static int ieee80211_key_enable_hw_accel(struct ieee80211_key *key)
 	if (!ret) {
 		key->flags |= KEY_FLAG_UPLOADED_TO_HARDWARE;
 
-		if (!((key->conf.flags & (IEEE80211_KEY_FLAG_GENERATE_MMIC |
-					   IEEE80211_KEY_FLAG_PUT_MIC_SPACE)) ||
-		      (key->conf.flags & IEEE80211_KEY_FLAG_RESERVE_TAILROOM)))
+		if (!(key->conf.flags & (IEEE80211_KEY_FLAG_GENERATE_MMIC |
+					 IEEE80211_KEY_FLAG_PUT_MIC_SPACE |
+					 IEEE80211_KEY_FLAG_RESERVE_TAILROOM)))
 			decrease_tailroom_need_count(sdata, 1);
 
 		WARN_ON((key->conf.flags & IEEE80211_KEY_FLAG_PUT_IV_SPACE) &&
@@ -242,9 +248,9 @@ static void ieee80211_key_disable_hw_accel(struct ieee80211_key *key)
 	sta = key->sta;
 	sdata = key->sdata;
 
-	if (!((key->conf.flags & (IEEE80211_KEY_FLAG_GENERATE_MMIC |
-				   IEEE80211_KEY_FLAG_PUT_MIC_SPACE)) ||
-	      (key->conf.flags & IEEE80211_KEY_FLAG_RESERVE_TAILROOM)))
+	if (!(key->conf.flags & (IEEE80211_KEY_FLAG_GENERATE_MMIC |
+				 IEEE80211_KEY_FLAG_PUT_MIC_SPACE |
+				 IEEE80211_KEY_FLAG_RESERVE_TAILROOM)))
 		increment_tailroom_need_count(sdata);
 
 	key->flags &= ~KEY_FLAG_UPLOADED_TO_HARDWARE;
@@ -1187,9 +1193,9 @@ void ieee80211_remove_key(struct ieee80211_key_conf *keyconf)
 	if (key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) {
 		key->flags &= ~KEY_FLAG_UPLOADED_TO_HARDWARE;
 
-		if (!((key->conf.flags & (IEEE80211_KEY_FLAG_GENERATE_MMIC |
-					   IEEE80211_KEY_FLAG_PUT_MIC_SPACE)) ||
-		      (key->conf.flags & IEEE80211_KEY_FLAG_RESERVE_TAILROOM)))
+		if (!(key->conf.flags & (IEEE80211_KEY_FLAG_GENERATE_MMIC |
+					 IEEE80211_KEY_FLAG_PUT_MIC_SPACE |
+					 IEEE80211_KEY_FLAG_RESERVE_TAILROOM)))
 			increment_tailroom_need_count(key->sdata);
 	}
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [RFC PATCH v3 02/12] nl80211/cfg80211: Extended Key ID support
  2019-02-10 21:06 [RFC PATCH v3 00/12] Draft for Extended Key ID support Alexander Wetzel
  2019-02-10 21:06 ` [RFC PATCH v3 01/12] mac80211: Optimize tailroom_needed update checks Alexander Wetzel
@ 2019-02-10 21:06 ` Alexander Wetzel
  2019-02-15 10:52   ` Johannes Berg
  2019-02-10 21:06 ` [RFC PATCH v3 03/12] mac80211: IEEE 802.11 " Alexander Wetzel
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 23+ messages in thread
From: Alexander Wetzel @ 2019-02-10 21:06 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Alexander Wetzel

Add support for IEEE 802.11-2016 "Extended Key ID for Individually
Addressed Frames".

Extends cfg80211 and nl80211 to allow pairwise keys to be installed Rx
only, switch Tx over separately and to use keyidx 1 for pairwise keys.

Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de>
---
 include/net/cfg80211.h       |  2 ++
 include/uapi/linux/nl80211.h | 23 ++++++++++++++++++++++-
 net/wireless/nl80211.c       | 32 ++++++++++++++++++++++++++++----
 net/wireless/rdev-ops.h      |  3 ++-
 net/wireless/trace.h         | 31 ++++++++++++++++++++++++++-----
 net/wireless/util.c          | 20 ++++++++++++++------
 6 files changed, 94 insertions(+), 17 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 7f2739a90bdb..71ebb3492e21 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -485,6 +485,7 @@ struct vif_params {
  *	with the get_key() callback, must be in little endian,
  *	length given by @seq_len.
  * @seq_len: length of @seq.
+ * @install_mode: key install mode (Rx/Tx, Rx only or set Tx)
  */
 struct key_params {
 	const u8 *key;
@@ -492,6 +493,7 @@ struct key_params {
 	int key_len;
 	int seq_len;
 	u32 cipher;
+	enum nl80211_key_install_mode install_mode;
 };
 
 /**
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index dd4f86ee286e..8ccede541913 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -4134,6 +4134,21 @@ enum nl80211_channel_type {
 	NL80211_CHAN_HT40PLUS
 };
 
+/**
+ * enum nl80211_key_install_mode - Key install mode
+ *
+ * @NL80211_KEY_RX_TX: Key must be installed for Rx and Tx
+ * @NL80211_KEY_RX_ONLY: Allowed in combination with @NL80211_CMD_NEW_KEY:
+ *	Unicast key has to be installed for Rx only.
+ * @NL80211_KEY_SWITCH_TX: Allowed in combination with @NL80211_CMD_SET_KEY:
+ *	Switch Tx to a Rx only, referenced by sta mac and idx.
+ */
+enum nl80211_key_install_mode {
+	NL80211_KEY_RX_TX,
+	NL80211_KEY_RX_ONLY,
+	NL80211_KEY_SWITCH_TX
+};
+
 /**
  * enum nl80211_chan_width - channel width definitions
  *
@@ -4377,6 +4392,9 @@ enum nl80211_key_default_types {
  * @NL80211_KEY_DEFAULT_TYPES: A nested attribute containing flags
  *	attributes, specifying what a key should be set as default as.
  *	See &enum nl80211_key_default_types.
+ * @NL80211_KEY_INSTALL_MODE: the install mode from
+ *	enum nl80211_key_install_mode. Defaults to @NL80211_KEY_RX_TX.
+ *
  * @__NL80211_KEY_AFTER_LAST: internal
  * @NL80211_KEY_MAX: highest key attribute
  */
@@ -4390,6 +4408,7 @@ enum nl80211_key_attributes {
 	NL80211_KEY_DEFAULT_MGMT,
 	NL80211_KEY_TYPE,
 	NL80211_KEY_DEFAULT_TYPES,
+	NL80211_KEY_INSTALL_MODE,
 
 	/* keep last */
 	__NL80211_KEY_AFTER_LAST,
@@ -5330,11 +5349,12 @@ enum nl80211_feature_flags {
  *	by the %NL80211_SCAN_FLAG_MIN_PREQ_CONTENT flag.
  * @NL80211_EXT_FEATURE_ENABLE_FTM_RESPONDER: Driver supports enabling fine
  *	timing measurement responder role.
- *
  * @NL80211_EXT_FEATURE_CAN_REPLACE_PTK0: Driver/device confirm that they are
  *      able to rekey an in-use key correctly. Userspace must not rekey PTK keys
  *      if this flag is not set. Ignoring this can leak clear text packets and/or
  *      freeze the connection.
+ * @NL80211_EXT_FEATURE_EXT_KEY_ID: Driver supports "Extended Key ID for
+ *      Individually Addressed Frames" from IEEE802.11-2016.
  *
  * @NL80211_EXT_FEATURE_AIRTIME_FAIRNESS: Driver supports getting airtime
  *	fairness for transmitted packets and has enabled airtime fairness
@@ -5384,6 +5404,7 @@ enum nl80211_ext_feature_index {
 	NL80211_EXT_FEATURE_ENABLE_FTM_RESPONDER,
 	NL80211_EXT_FEATURE_AIRTIME_FAIRNESS,
 	NL80211_EXT_FEATURE_AP_PMKSA_CACHING,
+	NL80211_EXT_FEATURE_EXT_KEY_ID,
 
 	/* add new features before the definition below */
 	NUM_NL80211_EXT_FEATURES,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index a3cc039b9f55..2a076b99737e 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -565,6 +565,7 @@ static const struct nla_policy nl80211_key_policy[NL80211_KEY_MAX + 1] = {
 	[NL80211_KEY_DEFAULT_MGMT] = { .type = NLA_FLAG },
 	[NL80211_KEY_TYPE] = NLA_POLICY_MAX(NLA_U32, NUM_NL80211_KEYTYPES - 1),
 	[NL80211_KEY_DEFAULT_TYPES] = { .type = NLA_NESTED },
+	[NL80211_KEY_INSTALL_MODE] = { .type = NLA_U8 },
 };
 
 /* policy for the key default flags */
@@ -970,6 +971,9 @@ static int nl80211_parse_key_new(struct genl_info *info, struct nlattr *key,
 		k->def_multi = kdt[NL80211_KEY_DEFAULT_TYPE_MULTICAST];
 	}
 
+	if (tb[NL80211_KEY_INSTALL_MODE])
+		k->p.install_mode = nla_get_u8(tb[NL80211_KEY_INSTALL_MODE]);
+
 	return 0;
 }
 
@@ -3646,8 +3650,11 @@ static int nl80211_set_key(struct sk_buff *skb, struct genl_info *info)
 	if (key.idx < 0)
 		return -EINVAL;
 
-	/* only support setting default key */
-	if (!key.def && !key.defmgmt)
+	/* Only support setting default key and
+	 * Extended Key ID action @NL80211_KEY_SWITCH_TX.
+	 */
+	if (!key.def && !key.defmgmt &&
+	    !(key.p.install_mode == NL80211_KEY_SWITCH_TX))
 		return -EINVAL;
 
 	wdev_lock(dev->ieee80211_ptr);
@@ -3671,7 +3678,7 @@ static int nl80211_set_key(struct sk_buff *skb, struct genl_info *info)
 #ifdef CONFIG_CFG80211_WEXT
 		dev->ieee80211_ptr->wext.default_key = key.idx;
 #endif
-	} else {
+	} else if (key.defmgmt) {
 		if (key.def_uni || !key.def_multi) {
 			err = -EINVAL;
 			goto out;
@@ -3693,8 +3700,25 @@ static int nl80211_set_key(struct sk_buff *skb, struct genl_info *info)
 #ifdef CONFIG_CFG80211_WEXT
 		dev->ieee80211_ptr->wext.default_mgmt_key = key.idx;
 #endif
-	}
+	} else if (key.p.install_mode == NL80211_KEY_SWITCH_TX &&
+		   wiphy_ext_feature_isset(&rdev->wiphy,
+					   NL80211_EXT_FEATURE_EXT_KEY_ID)) {
+		u8 *mac_addr = NULL;
 
+		if (info->attrs[NL80211_ATTR_MAC])
+			mac_addr = nla_data(info->attrs[NL80211_ATTR_MAC]);
+
+		if (!mac_addr || key.idx < 0 || key.idx > 1) {
+			err = -EINVAL;
+			goto out;
+		}
+
+		err = rdev_add_key(rdev, dev, key.idx,
+				   NL80211_KEYTYPE_PAIRWISE,
+				   mac_addr, &key.p);
+	} else {
+		err = -EINVAL;
+	}
  out:
 	wdev_unlock(dev->ieee80211_ptr);
 
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index 5cb48d135fab..4bf4e3774825 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -77,7 +77,8 @@ static inline int rdev_add_key(struct cfg80211_registered_device *rdev,
 			       struct key_params *params)
 {
 	int ret;
-	trace_rdev_add_key(&rdev->wiphy, netdev, key_index, pairwise, mac_addr);
+	trace_rdev_add_key(&rdev->wiphy, netdev, key_index, pairwise,
+			   mac_addr, params->install_mode);
 	ret = rdev->ops->add_key(&rdev->wiphy, netdev, key_index, pairwise,
 				  mac_addr, params);
 	trace_rdev_return_int(&rdev->wiphy, ret);
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 44b2ce1bb13a..b5c9e6729ff1 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -430,22 +430,43 @@ DECLARE_EVENT_CLASS(key_handle,
 		  BOOL_TO_STR(__entry->pairwise), MAC_PR_ARG(mac_addr))
 );
 
-DEFINE_EVENT(key_handle, rdev_add_key,
+DEFINE_EVENT(key_handle, rdev_get_key,
 	TP_PROTO(struct wiphy *wiphy, struct net_device *netdev, u8 key_index,
 		 bool pairwise, const u8 *mac_addr),
 	TP_ARGS(wiphy, netdev, key_index, pairwise, mac_addr)
 );
 
-DEFINE_EVENT(key_handle, rdev_get_key,
+DEFINE_EVENT(key_handle, rdev_del_key,
 	TP_PROTO(struct wiphy *wiphy, struct net_device *netdev, u8 key_index,
 		 bool pairwise, const u8 *mac_addr),
 	TP_ARGS(wiphy, netdev, key_index, pairwise, mac_addr)
 );
 
-DEFINE_EVENT(key_handle, rdev_del_key,
+TRACE_EVENT(rdev_add_key,
 	TP_PROTO(struct wiphy *wiphy, struct net_device *netdev, u8 key_index,
-		 bool pairwise, const u8 *mac_addr),
-	TP_ARGS(wiphy, netdev, key_index, pairwise, mac_addr)
+		 bool pairwise, const u8 *mac_addr, u8 install_mode),
+	TP_ARGS(wiphy, netdev, key_index, pairwise, mac_addr, install_mode),
+	TP_STRUCT__entry(
+		WIPHY_ENTRY
+		NETDEV_ENTRY
+		MAC_ENTRY(mac_addr)
+		__field(u8, key_index)
+		__field(bool, pairwise)
+		__field(u8, install_mode)
+	),
+	TP_fast_assign(
+		WIPHY_ASSIGN;
+		NETDEV_ASSIGN;
+		MAC_ASSIGN(mac_addr, mac_addr);
+		__entry->key_index = key_index;
+		__entry->pairwise = pairwise;
+		__entry->install_mode = install_mode;
+	),
+	TP_printk(WIPHY_PR_FMT ", " NETDEV_PR_FMT ", key_index: %u, "
+		  "install_mode: %u, pairwise: %s, mac addr: " MAC_PR_FMT,
+		  WIPHY_PR_ARG, NETDEV_PR_ARG, __entry->key_index,
+		  __entry->install_mode, BOOL_TO_STR(__entry->pairwise),
+		  MAC_PR_ARG(mac_addr))
 );
 
 TRACE_EVENT(rdev_set_default_key,
diff --git a/net/wireless/util.c b/net/wireless/util.c
index cd48cdd582c0..a3338e611190 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -236,14 +236,22 @@ int cfg80211_validate_key_settings(struct cfg80211_registered_device *rdev,
 	case WLAN_CIPHER_SUITE_CCMP_256:
 	case WLAN_CIPHER_SUITE_GCMP:
 	case WLAN_CIPHER_SUITE_GCMP_256:
-		/* Disallow pairwise keys with non-zero index unless it's WEP
-		 * or a vendor specific cipher (because current deployments use
-		 * pairwise WEP keys with non-zero indices and for vendor
-		 * specific ciphers this should be validated in the driver or
-		 * hardware level - but 802.11i clearly specifies to use zero)
+		/* IEEE802.11-2016 allows only 0 and - when using Extended Key
+		 * ID - 1 as index for pairwise keys.
+		 * @NL80211_KEY_RX_ONLY is only allowed for pairwise keys when
+		 * the driver supports Extended Key ID.
+		 * @NL80211_KEY_SWITCH_TX must not be set when validating a key.
 		 */
-		if (pairwise && key_idx)
+		if (params->install_mode == NL80211_KEY_RX_ONLY) {
+			if (!wiphy_ext_feature_isset(&rdev->wiphy,
+						     NL80211_EXT_FEATURE_EXT_KEY_ID))
+				return -EINVAL;
+			else if (!pairwise || key_idx < 0 || key_idx > 1)
+				return -EINVAL;
+		} else if ((pairwise && key_idx) ||
+			   params->install_mode == NL80211_KEY_SWITCH_TX) {
 			return -EINVAL;
+		}
 		break;
 	case WLAN_CIPHER_SUITE_AES_CMAC:
 	case WLAN_CIPHER_SUITE_BIP_CMAC_256:
-- 
2.20.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [RFC PATCH v3 03/12] mac80211: IEEE 802.11 Extended Key ID support
  2019-02-10 21:06 [RFC PATCH v3 00/12] Draft for Extended Key ID support Alexander Wetzel
  2019-02-10 21:06 ` [RFC PATCH v3 01/12] mac80211: Optimize tailroom_needed update checks Alexander Wetzel
  2019-02-10 21:06 ` [RFC PATCH v3 02/12] nl80211/cfg80211: Extended Key ID support Alexander Wetzel
@ 2019-02-10 21:06 ` " Alexander Wetzel
  2019-02-15 11:06   ` Johannes Berg
  2019-02-10 21:06 ` [RFC PATCH v3 04/12] mac80211: Compatibility " Alexander Wetzel
                   ` (9 subsequent siblings)
  12 siblings, 1 reply; 23+ messages in thread
From: Alexander Wetzel @ 2019-02-10 21:06 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Alexander Wetzel

Add support for Extended Key ID as defined in IEEE 802.11-2016.

 - Implement the nl80211 API for Extended Key ID
 - Extend mac80211 API to allow drivers to support Extended Key ID
 - Add handling for Rx-only keys (including tailroom_need_count)
 - Select the decryption key based on the MPDU keyid
 - Enforce cipher does not change when replacing a key.

Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de>
---
 include/net/mac80211.h     |  19 ++++-
 net/mac80211/cfg.c         |  38 ++++++++++
 net/mac80211/debugfs.c     |   1 +
 net/mac80211/ieee80211_i.h |   2 +-
 net/mac80211/key.c         | 138 +++++++++++++++++++++++++++++--------
 net/mac80211/key.h         |   4 ++
 net/mac80211/main.c        |   5 ++
 net/mac80211/rx.c          |  74 ++++++++++----------
 net/mac80211/sta_info.c    |   9 +++
 net/mac80211/sta_info.h    |   2 +-
 net/mac80211/tx.c          |  13 +---
 11 files changed, 227 insertions(+), 78 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index de866a7253c9..e16bc7623dc0 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1804,13 +1804,22 @@ struct ieee80211_cipher_scheme {
  * enum set_key_cmd - key command
  *
  * Used with the set_key() callback in &struct ieee80211_ops, this
- * indicates whether a key is being removed or added.
+ * indicates which action has to be performed with the key.
  *
- * @SET_KEY: a key is set
+ * @SET_KEY: a key is set and valid for Rx and Tx immediately
  * @DISABLE_KEY: a key must be disabled
+ *
+ * Additional commands for drivers supporting Extended Key ID:
+ *
+ * @EXT_SET_KEY: a new key must be set but is only valid for decryption
+ * @EXT_KEY_RX_TX: a key installed with @EXT_SET_KEY is becoming the
+ *	designated Rx/Tx key for the station
  */
 enum set_key_cmd {
-	SET_KEY, DISABLE_KEY,
+	SET_KEY,
+	DISABLE_KEY,
+	EXT_SET_KEY,
+	EXT_KEY_RX_TX,
 };
 
 /**
@@ -2219,6 +2228,9 @@ struct ieee80211_txq {
  * @IEEE80211_HW_TX_STATUS_NO_AMPDU_LEN: Driver does not report accurate A-MPDU
  *	length in tx status information
  *
+ * @IEEE80211_HW_EXT_KEY_ID_NATIVE: Driver and hardware are supporting Extended
+ *	Key ID and can handle two unicast keys per station for Rx and Tx.
+ *
  * @NUM_IEEE80211_HW_FLAGS: number of hardware flags, used for sizing arrays
  */
 enum ieee80211_hw_flags {
@@ -2268,6 +2280,7 @@ enum ieee80211_hw_flags {
 	IEEE80211_HW_SUPPORTS_VHT_EXT_NSS_BW,
 	IEEE80211_HW_STA_MMPDU_TXQ,
 	IEEE80211_HW_TX_STATUS_NO_AMPDU_LEN,
+	IEEE80211_HW_EXT_KEY_ID_NATIVE,
 
 	/* keep last, obviously */
 	NUM_IEEE80211_HW_FLAGS
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index d65aa019ce85..a032da64eed2 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -351,6 +351,38 @@ static int ieee80211_set_noack_map(struct wiphy *wiphy,
 	return 0;
 }
 
+static int ieee80211_set_tx_key(struct ieee80211_sub_if_data *sdata,
+				const u8 *mac_addr, u8 key_idx)
+{
+	struct ieee80211_local *local = sdata->local;
+	struct ieee80211_key *key;
+	struct sta_info *sta;
+	int ret;
+
+	if (!wiphy_ext_feature_isset(local->hw.wiphy,
+				     NL80211_EXT_FEATURE_EXT_KEY_ID))
+		return -EINVAL;
+
+	sta = sta_info_get_bss(sdata, mac_addr);
+
+	if (!sta)
+		return -EINVAL;
+
+	if (sta->ptk_idx == key_idx)
+		return 0;
+
+	mutex_lock(&local->key_mtx);
+	key = key_mtx_dereference(local, sta->ptk[key_idx]);
+
+	if (key && key->flags & KEY_FLAG_RX_ONLY)
+		ret = ieee80211_key_activate_tx(key);
+	else
+		ret = -EINVAL;
+
+	mutex_unlock(&local->key_mtx);
+	return ret;
+}
+
 static int ieee80211_add_key(struct wiphy *wiphy, struct net_device *dev,
 			     u8 key_idx, bool pairwise, const u8 *mac_addr,
 			     struct key_params *params)
@@ -365,6 +397,9 @@ static int ieee80211_add_key(struct wiphy *wiphy, struct net_device *dev,
 	if (!ieee80211_sdata_running(sdata))
 		return -ENETDOWN;
 
+	if (pairwise && params->install_mode == NL80211_KEY_SWITCH_TX)
+		return ieee80211_set_tx_key(sdata, mac_addr, key_idx);
+
 	/* reject WEP and TKIP keys if WEP failed to initialize */
 	switch (params->cipher) {
 	case WLAN_CIPHER_SUITE_WEP40:
@@ -396,6 +431,9 @@ static int ieee80211_add_key(struct wiphy *wiphy, struct net_device *dev,
 	if (pairwise)
 		key->conf.flags |= IEEE80211_KEY_FLAG_PAIRWISE;
 
+	if (params->install_mode == NL80211_KEY_RX_ONLY)
+		key->flags |= KEY_FLAG_RX_ONLY;
+
 	mutex_lock(&local->sta_mtx);
 
 	if (mac_addr) {
diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
index 343ad0a915e4..334a9883894f 100644
--- a/net/mac80211/debugfs.c
+++ b/net/mac80211/debugfs.c
@@ -219,6 +219,7 @@ static const char *hw_flag_names[] = {
 	FLAG(SUPPORTS_VHT_EXT_NSS_BW),
 	FLAG(STA_MMPDU_TXQ),
 	FLAG(TX_STATUS_NO_AMPDU_LEN),
+	FLAG(EXT_KEY_ID_NATIVE),
 #undef FLAG
 };
 
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 056b16bce3b0..cbc35a31adc5 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1263,7 +1263,7 @@ struct ieee80211_local {
 
 	/*
 	 * Key mutex, protects sdata's key_list and sta_info's
-	 * key pointers (write access, they're RCU.)
+	 * key pointers and @ptk_idx (write access, they're RCU.)
 	 */
 	struct mutex key_mtx;
 
diff --git a/net/mac80211/key.c b/net/mac80211/key.c
index b9f2bfc00263..d91503de1e1d 100644
--- a/net/mac80211/key.c
+++ b/net/mac80211/key.c
@@ -127,8 +127,13 @@ static void decrease_tailroom_need_count(struct ieee80211_sub_if_data *sdata,
 static int ieee80211_key_enable_hw_accel(struct ieee80211_key *key)
 {
 	struct ieee80211_sub_if_data *sdata = key->sdata;
+	struct ieee80211_local *local = key->local;
 	struct sta_info *sta;
+	bool rx_only = key->flags & KEY_FLAG_RX_ONLY;
+	bool pairwise = key->conf.flags & IEEE80211_KEY_FLAG_PAIRWISE;
+	bool ext_native = ieee80211_hw_check(&local->hw, EXT_KEY_ID_NATIVE);
 	int ret = -EOPNOTSUPP;
+	int cmd;
 
 	might_sleep();
 
@@ -150,10 +155,10 @@ static int ieee80211_key_enable_hw_accel(struct ieee80211_key *key)
 		return -EINVAL;
 	}
 
-	if (!key->local->ops->set_key)
+	if (!local->ops->set_key)
 		goto out_unsupported;
 
-	assert_key_lock(key->local);
+	assert_key_lock(local);
 
 	sta = key->sta;
 
@@ -161,8 +166,8 @@ static int ieee80211_key_enable_hw_accel(struct ieee80211_key *key)
 	 * If this is a per-STA GTK, check if it
 	 * is supported; if not, return.
 	 */
-	if (sta && !(key->conf.flags & IEEE80211_KEY_FLAG_PAIRWISE) &&
-	    !ieee80211_hw_check(&key->local->hw, SUPPORTS_PER_STA_GTK))
+	if (sta && !pairwise &&
+	    !ieee80211_hw_check(&local->hw, SUPPORTS_PER_STA_GTK))
 		goto out_unsupported;
 
 	if (sta && !sta->uploaded)
@@ -173,19 +178,25 @@ static int ieee80211_key_enable_hw_accel(struct ieee80211_key *key)
 		 * The driver doesn't know anything about VLAN interfaces.
 		 * Hence, don't send GTKs for VLAN interfaces to the driver.
 		 */
-		if (!(key->conf.flags & IEEE80211_KEY_FLAG_PAIRWISE)) {
+		if (!pairwise) {
 			ret = 1;
 			goto out_unsupported;
 		}
 	}
 
-	ret = drv_set_key(key->local, SET_KEY, sdata,
+	if (rx_only)
+		cmd = EXT_SET_KEY;
+	else
+		cmd = SET_KEY;
+
+	ret = drv_set_key(local, cmd, sdata,
 			  sta ? &sta->sta : NULL, &key->conf);
 
 	if (!ret) {
 		key->flags |= KEY_FLAG_UPLOADED_TO_HARDWARE;
 
-		if (!(key->conf.flags & (IEEE80211_KEY_FLAG_GENERATE_MMIC |
+		if (!(rx_only ||
+		      key->conf.flags & (IEEE80211_KEY_FLAG_GENERATE_MMIC |
 					 IEEE80211_KEY_FLAG_PUT_MIC_SPACE |
 					 IEEE80211_KEY_FLAG_RESERVE_TAILROOM)))
 			decrease_tailroom_need_count(sdata, 1);
@@ -221,7 +232,7 @@ static int ieee80211_key_enable_hw_accel(struct ieee80211_key *key)
 		/* all of these we can do in software - if driver can */
 		if (ret == 1)
 			return 0;
-		if (ieee80211_hw_check(&key->local->hw, SW_CRYPTO_CONTROL))
+		if (ieee80211_hw_check(&local->hw, SW_CRYPTO_CONTROL))
 			return -EINVAL;
 		return 0;
 	default:
@@ -248,7 +259,8 @@ static void ieee80211_key_disable_hw_accel(struct ieee80211_key *key)
 	sta = key->sta;
 	sdata = key->sdata;
 
-	if (!(key->conf.flags & (IEEE80211_KEY_FLAG_GENERATE_MMIC |
+	if (!(key->flags & KEY_FLAG_RX_ONLY ||
+	      key->conf.flags & (IEEE80211_KEY_FLAG_GENERATE_MMIC |
 				 IEEE80211_KEY_FLAG_PUT_MIC_SPACE |
 				 IEEE80211_KEY_FLAG_RESERVE_TAILROOM)))
 		increment_tailroom_need_count(sdata);
@@ -264,9 +276,55 @@ static void ieee80211_key_disable_hw_accel(struct ieee80211_key *key)
 			  sta ? sta->sta.addr : bcast_addr, ret);
 }
 
+int ieee80211_key_activate_tx(struct ieee80211_key *key)
+{
+	struct ieee80211_sub_if_data *sdata = key->sdata;
+	struct sta_info *sta = key->sta;
+	struct ieee80211_local *local = key->local;
+	struct ieee80211_key *old;
+	int ret;
+
+	assert_key_lock(local);
+
+	key->flags &= ~KEY_FLAG_RX_ONLY;
+
+	if (!(key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) ||
+	    key->conf.flags & (IEEE80211_KEY_FLAG_GENERATE_MMIC |
+			       IEEE80211_KEY_FLAG_PUT_MIC_SPACE |
+			       IEEE80211_KEY_FLAG_RESERVE_TAILROOM))
+		increment_tailroom_need_count(sdata);
+
+	if (key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) {
+		ret = drv_set_key(local, EXT_KEY_RX_TX, sdata,
+				  &sta->sta, &key->conf);
+		if (ret) {
+			sdata_err(sdata,
+				  "failed to activate key for Tx (%d, %pM)\n",
+				  key->conf.keyidx, sta->sta.addr);
+			return ret;
+		}
+	}
+
+	old = key_mtx_dereference(local, sta->ptk[sta->ptk_idx]);
+	sta->ptk_idx = key->conf.keyidx;
+	ieee80211_check_fast_xmit(sta);
+
+	if (old) {
+		old->flags |= KEY_FLAG_RX_ONLY;
+
+		if (!(old->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) ||
+		    old->conf.flags & (IEEE80211_KEY_FLAG_GENERATE_MMIC |
+				       IEEE80211_KEY_FLAG_PUT_MIC_SPACE |
+				       IEEE80211_KEY_FLAG_RESERVE_TAILROOM))
+			decrease_tailroom_need_count(sdata, 1);
+	}
+
+	return 0;
+}
+
 static int ieee80211_hw_key_replace(struct ieee80211_key *old_key,
 				    struct ieee80211_key *new_key,
-				    bool ptk0rekey)
+				    bool pairwise)
 {
 	struct ieee80211_sub_if_data *sdata;
 	struct ieee80211_local *local;
@@ -283,16 +341,17 @@ static int ieee80211_hw_key_replace(struct ieee80211_key *old_key,
 	assert_key_lock(old_key->local);
 	sta = old_key->sta;
 
-	/* PTK only using key ID 0 needs special handling on rekey */
-	if (new_key && sta && ptk0rekey) {
+	/* Unicast rekey without Extended Key ID needs special handling */
+	if (new_key && pairwise && sta &&
+	    rcu_access_pointer(sta->ptk[sta->ptk_idx]) == old_key) {
 		local = old_key->local;
 		sdata = old_key->sdata;
 
-		/* Stop TX till we are on the new key */
+		/* Stop Tx till we are on the new key */
 		old_key->flags |= KEY_FLAG_TAINTED;
 		ieee80211_clear_fast_xmit(sta);
 
-		/* Aggregation sessions during rekey are complicated due to the
+		/* Aggregation sessions during rekey are complicated by the
 		 * reorder buffer and retransmits. Side step that by blocking
 		 * aggregation during rekey and tear down running sessions.
 		 */
@@ -400,10 +459,6 @@ static int ieee80211_key_replace(struct ieee80211_sub_if_data *sdata,
 
 	if (old) {
 		idx = old->conf.keyidx;
-		/* TODO: proper implement and test "Extended Key ID for
-		 * Individually Addressed Frames" from IEEE 802.11-2016.
-		 * Till then always assume only key ID 0 is used for
-		 * pairwise keys.*/
 		ret = ieee80211_hw_key_replace(old, new, pairwise);
 	} else {
 		/* new must be provided in case old is not */
@@ -420,15 +475,19 @@ static int ieee80211_key_replace(struct ieee80211_sub_if_data *sdata,
 	if (sta) {
 		if (pairwise) {
 			rcu_assign_pointer(sta->ptk[idx], new);
-			sta->ptk_idx = idx;
-			if (new) {
+			if (new && !(new->flags & KEY_FLAG_RX_ONLY)) {
+				sta->ptk_idx = idx;
 				clear_sta_flag(sta, WLAN_STA_BLOCK_BA);
 				ieee80211_check_fast_xmit(sta);
 			}
 		} else {
 			rcu_assign_pointer(sta->gtk[idx], new);
 		}
-		if (new)
+		/* Only needed when transition from no key -> key.
+		 * Still triggers unnecessary when using Extended Key ID
+		 * and installing the second key ID the first time.
+		 */
+		if (new && !old)
 			ieee80211_check_fast_rx(sta);
 	} else {
 		defunikey = old &&
@@ -664,6 +723,9 @@ static void __ieee80211_key_destroy(struct ieee80211_key *key,
 
 		ieee80211_debugfs_key_remove(key);
 
+		if (key->flags & KEY_FLAG_RX_ONLY)
+			return;
+
 		if (delay_tailroom) {
 			/* see ieee80211_delayed_tailroom_dec */
 			sdata->crypto_tx_tailroom_pending_dec++;
@@ -744,16 +806,33 @@ int ieee80211_key_link(struct ieee80211_key *key,
 	 * can cause warnings to appear.
 	 */
 	bool delay_tailroom = sdata->vif.type == NL80211_IFTYPE_STATION;
-	int ret;
+	bool rx_only = key->flags & KEY_FLAG_RX_ONLY;
+	int ret = -EOPNOTSUPP;
 
 	mutex_lock(&sdata->local->key_mtx);
 
-	if (sta && pairwise)
+	if (sta && pairwise) {
+		struct ieee80211_key *alt_key;
+
 		old_key = key_mtx_dereference(sdata->local, sta->ptk[idx]);
-	else if (sta)
+		alt_key = key_mtx_dereference(sdata->local, sta->ptk[idx ^ 1]);
+
+		/* Don't allow pairwise keys to change cipher on rekey */
+		if (key &&
+		    ((alt_key && alt_key->conf.cipher != key->conf.cipher) ||
+		     (old_key && old_key->conf.cipher != key->conf.cipher)))
+			goto out;
+	} else if (sta) {
 		old_key = key_mtx_dereference(sdata->local, sta->gtk[idx]);
-	else
+	} else {
 		old_key = key_mtx_dereference(sdata->local, sdata->keys[idx]);
+	}
+
+	/* Don't allow non-pairwise keys to change cipher on rekey */
+	if (!pairwise) {
+		if (key && old_key && old_key->conf.cipher != key->conf.cipher)
+			goto out;
+	}
 
 	/*
 	 * Silently accept key re-installation without really installing the
@@ -769,7 +848,8 @@ int ieee80211_key_link(struct ieee80211_key *key,
 	key->sdata = sdata;
 	key->sta = sta;
 
-	increment_tailroom_need_count(sdata);
+	if (!rx_only)
+		increment_tailroom_need_count(sdata);
 
 	ret = ieee80211_key_replace(sdata, sta, pairwise, old_key, key);
 
@@ -823,7 +903,8 @@ void ieee80211_enable_keys(struct ieee80211_sub_if_data *sdata)
 	}
 
 	list_for_each_entry(key, &sdata->key_list, list) {
-		increment_tailroom_need_count(sdata);
+		if (!(key->flags & KEY_FLAG_RX_ONLY))
+			increment_tailroom_need_count(sdata);
 		ieee80211_key_enable_hw_accel(key);
 	}
 
@@ -1193,7 +1274,8 @@ void ieee80211_remove_key(struct ieee80211_key_conf *keyconf)
 	if (key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) {
 		key->flags &= ~KEY_FLAG_UPLOADED_TO_HARDWARE;
 
-		if (!(key->conf.flags & (IEEE80211_KEY_FLAG_GENERATE_MMIC |
+		if (!(key->flags & KEY_FLAG_RX_ONLY ||
+		      key->conf.flags & (IEEE80211_KEY_FLAG_GENERATE_MMIC |
 					 IEEE80211_KEY_FLAG_PUT_MIC_SPACE |
 					 IEEE80211_KEY_FLAG_RESERVE_TAILROOM)))
 			increment_tailroom_need_count(key->sdata);
diff --git a/net/mac80211/key.h b/net/mac80211/key.h
index ebdb80b85dc3..1a3da999e0c4 100644
--- a/net/mac80211/key.h
+++ b/net/mac80211/key.h
@@ -18,6 +18,7 @@
 
 #define NUM_DEFAULT_KEYS 4
 #define NUM_DEFAULT_MGMT_KEYS 2
+#define INVALID_PTK_KEYIDX 2 /* Existing key slot never used by PTK keys */
 
 struct ieee80211_local;
 struct ieee80211_sub_if_data;
@@ -30,11 +31,13 @@ struct sta_info;
  *	in the hardware for TX crypto hardware acceleration.
  * @KEY_FLAG_TAINTED: Key is tainted and packets should be dropped.
  * @KEY_FLAG_CIPHER_SCHEME: This key is for a hardware cipher scheme
+ * @KEY_FLAG_RX_ONLY: Pairwise key only allowed to be used on Rx.
  */
 enum ieee80211_internal_key_flags {
 	KEY_FLAG_UPLOADED_TO_HARDWARE	= BIT(0),
 	KEY_FLAG_TAINTED		= BIT(1),
 	KEY_FLAG_CIPHER_SCHEME		= BIT(2),
+	KEY_FLAG_RX_ONLY		= BIT(3),
 };
 
 enum ieee80211_internal_tkip_state {
@@ -146,6 +149,7 @@ ieee80211_key_alloc(u32 cipher, int idx, size_t key_len,
 int ieee80211_key_link(struct ieee80211_key *key,
 		       struct ieee80211_sub_if_data *sdata,
 		       struct sta_info *sta);
+int ieee80211_key_activate_tx(struct ieee80211_key *key);
 void ieee80211_key_free(struct ieee80211_key *key, bool delay_tailroom);
 void ieee80211_key_free_unused(struct ieee80211_key *key);
 void ieee80211_set_default_key(struct ieee80211_sub_if_data *sdata, int idx,
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 71005b6dfcd1..ea34544985f3 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -1051,6 +1051,11 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
 		}
 	}
 
+	/* mac80211 supports Extended Key ID when driver does */
+	if (ieee80211_hw_check(&local->hw, EXT_KEY_ID_NATIVE))
+		wiphy_ext_feature_set(local->hw.wiphy,
+				      NL80211_EXT_FEATURE_EXT_KEY_ID);
+
 	/*
 	 * Calculate scan IE length -- we need this to alloc
 	 * memory and to subtract from the driver limit. It
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index bb4d71efb6fb..ce786311baf4 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -988,23 +988,43 @@ static int ieee80211_get_mmie_keyidx(struct sk_buff *skb)
 	return -1;
 }
 
-static int ieee80211_get_cs_keyid(const struct ieee80211_cipher_scheme *cs,
-				  struct sk_buff *skb)
+static int ieee80211_get_keyid(struct sk_buff *skb,
+			       const struct ieee80211_cipher_scheme *cs)
 {
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
 	__le16 fc;
 	int hdrlen;
+	int minlen;
+	u8 key_idx_off;
+	u8 key_idx_shift;
 	u8 keyid;
 
 	fc = hdr->frame_control;
 	hdrlen = ieee80211_hdrlen(fc);
 
-	if (skb->len < hdrlen + cs->hdr_len)
+	if (cs) {
+		minlen = hdrlen + cs->hdr_len;
+		key_idx_off = hdrlen + cs->key_idx_off;
+		key_idx_shift = cs->key_idx_shift;
+	} else {
+		/* WEP, TKIP, CCMP and GCMP have the key id at the same place */
+		minlen = hdrlen + IEEE80211_WEP_IV_LEN;
+		key_idx_off = hdrlen + 3;
+		key_idx_shift = 6;
+	}
+
+	if (unlikely(skb->len < minlen))
 		return -EINVAL;
 
-	skb_copy_bits(skb, hdrlen + cs->key_idx_off, &keyid, 1);
-	keyid &= cs->key_idx_mask;
-	keyid >>= cs->key_idx_shift;
+	skb_copy_bits(skb, key_idx_off, &keyid, 1);
+
+	if (cs)
+		keyid &= cs->key_idx_mask;
+	keyid >>= key_idx_shift;
+
+	/* cs could use more than the usual two bits for the keyid */
+	if (unlikely(keyid > NUM_DEFAULT_KEYS))
+		return -EINVAL;
 
 	return keyid;
 }
@@ -1835,9 +1855,9 @@ ieee80211_rx_h_decrypt(struct ieee80211_rx_data *rx)
 	struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
 	int keyidx;
-	int hdrlen;
 	ieee80211_rx_result result = RX_DROP_UNUSABLE;
 	struct ieee80211_key *sta_ptk = NULL;
+	struct ieee80211_key *ptk_idx = NULL;
 	int mmie_keyidx = -1;
 	__le16 fc;
 	const struct ieee80211_cipher_scheme *cs = NULL;
@@ -1875,21 +1895,24 @@ ieee80211_rx_h_decrypt(struct ieee80211_rx_data *rx)
 
 	if (rx->sta) {
 		int keyid = rx->sta->ptk_idx;
+		sta_ptk = rcu_dereference(rx->sta->ptk[keyid]);
 
-		if (ieee80211_has_protected(fc) && rx->sta->cipher_scheme) {
+		if (ieee80211_has_protected(fc)) {
 			cs = rx->sta->cipher_scheme;
-			keyid = ieee80211_get_cs_keyid(cs, rx->skb);
+			keyid = ieee80211_get_keyid(rx->skb, cs);
+
 			if (unlikely(keyid < 0))
 				return RX_DROP_UNUSABLE;
+
+			ptk_idx = rcu_dereference(rx->sta->ptk[keyid]);
 		}
-		sta_ptk = rcu_dereference(rx->sta->ptk[keyid]);
 	}
 
 	if (!ieee80211_has_protected(fc))
 		mmie_keyidx = ieee80211_get_mmie_keyidx(rx->skb);
 
 	if (!is_multicast_ether_addr(hdr->addr1) && sta_ptk) {
-		rx->key = sta_ptk;
+		rx->key = ptk_idx ? ptk_idx : sta_ptk;
 		if ((status->flag & RX_FLAG_DECRYPTED) &&
 		    (status->flag & RX_FLAG_IV_STRIPPED))
 			return RX_CONTINUE;
@@ -1949,8 +1972,6 @@ ieee80211_rx_h_decrypt(struct ieee80211_rx_data *rx)
 		}
 		return RX_CONTINUE;
 	} else {
-		u8 keyid;
-
 		/*
 		 * The device doesn't give us the IV so we won't be
 		 * able to look up the key. That's ok though, we
@@ -1964,23 +1985,10 @@ ieee80211_rx_h_decrypt(struct ieee80211_rx_data *rx)
 		    (status->flag & RX_FLAG_IV_STRIPPED))
 			return RX_CONTINUE;
 
-		hdrlen = ieee80211_hdrlen(fc);
-
-		if (cs) {
-			keyidx = ieee80211_get_cs_keyid(cs, rx->skb);
+		keyidx = ieee80211_get_keyid(rx->skb, cs);
 
-			if (unlikely(keyidx < 0))
-				return RX_DROP_UNUSABLE;
-		} else {
-			if (rx->skb->len < 8 + hdrlen)
-				return RX_DROP_UNUSABLE; /* TODO: count this? */
-			/*
-			 * no need to call ieee80211_wep_get_keyidx,
-			 * it verifies a bunch of things we've done already
-			 */
-			skb_copy_bits(rx->skb, hdrlen + 3, &keyid, 1);
-			keyidx = keyid >> 6;
-		}
+		if (unlikely(keyidx < 0))
+			return RX_DROP_UNUSABLE;
 
 		/* check per-station GTK first, if multicast packet */
 		if (is_multicast_ether_addr(hdr->addr1) && rx->sta)
@@ -4020,12 +4028,8 @@ void ieee80211_check_fast_rx(struct sta_info *sta)
 		case WLAN_CIPHER_SUITE_GCMP_256:
 			break;
 		default:
-			/* we also don't want to deal with WEP or cipher scheme
-			 * since those require looking up the key idx in the
-			 * frame, rather than assuming the PTK is used
-			 * (we need to revisit this once we implement the real
-			 * PTK index, which is now valid in the spec, but we
-			 * haven't implemented that part yet)
+			/* We also don't want to deal with
+			 * WEP or cipher scheme.
 			 */
 			goto clear_rcu;
 		}
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 11f058987a54..09c69955c6e3 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -347,6 +347,15 @@ struct sta_info *sta_info_alloc(struct ieee80211_sub_if_data *sdata,
 	sta->sta.max_rx_aggregation_subframes =
 		local->hw.max_rx_aggregation_subframes;
 
+	/* Extended Key ID can install keys for keyid 0 and 1 as Rx only.
+	 * Tx starts uses a key as soon as a key is installed in the slot
+	 * ptk_idx references to. To avoid using the initial Rx key prematurely
+	 * for Tx we initialize ptk_idx to a value never used, making sure the
+	 * referenced key is always NULL till ptk_idx is set to a valid value.
+	 */
+	BUILD_BUG_ON(ARRAY_SIZE(sta->ptk) <= INVALID_PTK_KEYIDX);
+	sta->ptk_idx = INVALID_PTK_KEYIDX;
+
 	sta->local = local;
 	sta->sdata = sdata;
 	sta->rx_stats.last_rx = jiffies;
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 71f7e4973329..304a7ea24757 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -449,7 +449,7 @@ struct ieee80211_sta_rx_stats {
  * @local: pointer to the global information
  * @sdata: virtual interface this station belongs to
  * @ptk: peer keys negotiated with this station, if any
- * @ptk_idx: last installed peer key index
+ * @ptk_idx: peer key index to use for transmissions
  * @gtk: group keys negotiated with this station, if any
  * @rate_ctrl: rate control algorithm reference
  * @rate_ctrl_lock: spinlock used to protect rate control data
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 8a49a74c0a37..111bd6c490a6 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3000,23 +3000,15 @@ void ieee80211_check_fast_xmit(struct sta_info *sta)
 		switch (build.key->conf.cipher) {
 		case WLAN_CIPHER_SUITE_CCMP:
 		case WLAN_CIPHER_SUITE_CCMP_256:
-			/* add fixed key ID */
-			if (gen_iv) {
-				(build.hdr + build.hdr_len)[3] =
-					0x20 | (build.key->conf.keyidx << 6);
+			if (gen_iv)
 				build.pn_offs = build.hdr_len;
-			}
 			if (gen_iv || iv_spc)
 				build.hdr_len += IEEE80211_CCMP_HDR_LEN;
 			break;
 		case WLAN_CIPHER_SUITE_GCMP:
 		case WLAN_CIPHER_SUITE_GCMP_256:
-			/* add fixed key ID */
-			if (gen_iv) {
-				(build.hdr + build.hdr_len)[3] =
-					0x20 | (build.key->conf.keyidx << 6);
+			if (gen_iv)
 				build.pn_offs = build.hdr_len;
-			}
 			if (gen_iv || iv_spc)
 				build.hdr_len += IEEE80211_GCMP_HDR_LEN;
 			break;
@@ -3383,6 +3375,7 @@ static void ieee80211_xmit_fast_finish(struct ieee80211_sub_if_data *sdata,
 			pn = atomic64_inc_return(&key->conf.tx_pn);
 			crypto_hdr[0] = pn;
 			crypto_hdr[1] = pn >> 8;
+			crypto_hdr[3] = 0x20 | (key->conf.keyidx << 6);
 			crypto_hdr[4] = pn >> 16;
 			crypto_hdr[5] = pn >> 24;
 			crypto_hdr[6] = pn >> 32;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [RFC PATCH v3 04/12] mac80211: Compatibility Extended Key ID support
  2019-02-10 21:06 [RFC PATCH v3 00/12] Draft for Extended Key ID support Alexander Wetzel
                   ` (2 preceding siblings ...)
  2019-02-10 21:06 ` [RFC PATCH v3 03/12] mac80211: IEEE 802.11 " Alexander Wetzel
@ 2019-02-10 21:06 ` " Alexander Wetzel
  2019-02-15 11:09   ` Johannes Berg
  2019-02-10 21:06 ` [RFC PATCH v3 05/12] mac80211: Mark A-MPDU keyid borders for drivers Alexander Wetzel
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 23+ messages in thread
From: Alexander Wetzel @ 2019-02-10 21:06 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Alexander Wetzel

Allow drivers to support Extended Key ID when they are not able to
handle two unicast keys per station for Rx by falling back to software
decryption when replacing keys.

Rx HW decryption activation is delayed till we either get the first MPDU
encrypted with the new key or for max 10s.

Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de>
---

Most drivers with the exception of ath10k should be able to support this
mode even without firmware updates from the vendors. (ath10k seems to be
a no-go without some firmware update, but that's not for discussion
here.)
Biggest downside are - besides the added complexity - potential severe
CPU peaks. (Imagine an rebooted AP with hundred of clients using
it with rekeying enabled and a weak CPU.) 

Also noteworthy is, that using HW Rx decryption here at all is kind of
breaking the standard: As soon as one Rx key is offloaded to the
hardware we are/may no longer be able to decrypt packets with the "other"
valid keyid. (The hardware may have tried to decrypt it with the wrong
key and then hands over the decryption failure instead of the original
data.) So this will only work correctly as long as the remote sta is
switching to the new keyid at a time within 10s and not mixing old and
new keyids for a while. If that assumption breaks, mac80211 will not be
able to decrypt mangled packets.
That said it seems unlikely that this could be much more than some
retransmits and I get perfect looking rekeys in my test setup with max a
few dozen packets decrypted in software.

 include/net/mac80211.h  | 46 ++++++++++++++++++++++++++++
 net/mac80211/debugfs.c  |  1 +
 net/mac80211/key.c      | 67 +++++++++++++++++++++++++++++++++++++++--
 net/mac80211/key.h      |  4 +++
 net/mac80211/main.c     |  3 +-
 net/mac80211/rx.c       |  7 +++++
 net/mac80211/sta_info.c |  3 ++
 net/mac80211/sta_info.h |  2 ++
 8 files changed, 129 insertions(+), 4 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index e16bc7623dc0..eafad5eb8953 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1814,12 +1814,16 @@ struct ieee80211_cipher_scheme {
  * @EXT_SET_KEY: a new key must be set but is only valid for decryption
  * @EXT_KEY_RX_TX: a key installed with @EXT_SET_KEY is becoming the
  *	designated Rx/Tx key for the station
+ * @EXT_DISABLE_KEY_RX: installed key must switch to to software decryption
+ *	while not altering Tx encryption. Command only required when driver
+ *	is using EXT_KEY_ID_COMPAT for Extended Key ID support.
  */
 enum set_key_cmd {
 	SET_KEY,
 	DISABLE_KEY,
 	EXT_SET_KEY,
 	EXT_KEY_RX_TX,
+	EXT_DISABLE_KEY_RX,
 };
 
 /**
@@ -2231,6 +2235,10 @@ struct ieee80211_txq {
  * @IEEE80211_HW_EXT_KEY_ID_NATIVE: Driver and hardware are supporting Extended
  *	Key ID and can handle two unicast keys per station for Rx and Tx.
  *
+ * @IEEE80211_HW_EXT_KEY_ID_COMPAT: Driver and hardware support Extended Key ID
+ *	when mac80211 handles Rx decryption during transition from one keyid to
+ *	the next.
+ *
  * @NUM_IEEE80211_HW_FLAGS: number of hardware flags, used for sizing arrays
  */
 enum ieee80211_hw_flags {
@@ -2281,6 +2289,7 @@ enum ieee80211_hw_flags {
 	IEEE80211_HW_STA_MMPDU_TXQ,
 	IEEE80211_HW_TX_STATUS_NO_AMPDU_LEN,
 	IEEE80211_HW_EXT_KEY_ID_NATIVE,
+	IEEE80211_HW_EXT_KEY_ID_COMPAT,
 
 	/* keep last, obviously */
 	NUM_IEEE80211_HW_FLAGS
@@ -2641,6 +2650,43 @@ void ieee80211_free_txskb(struct ieee80211_hw *hw, struct sk_buff *skb);
    Mac80211 will not queue any new frames for a deleted key to the driver.
  */
 
+/**
+ * DOC: Extended Key ID support
+ *
+ * Mac80211 can support "Extended Key ID" from IEEE 802.11-2016, allowing to
+ * rekey the in-use unicast key with ideally zero impact for ongoing
+ * transmissions.
+ *
+ * There are two options for a driver to support Extended Key ID:
+ * 1) Native:
+ *    The "Native" Extended Key ID mode is using the key commands
+ *    %EXT_SET_KEY, %EXT_KEY_RX_TX and %DISABLE_KEY.
+ *    Drivers/cards fully compatible can set @IEEE80211_HW_EXT_KEY_ID_NATIVE,
+ *    allowing mac80211 to install two unicast keys per station to the driver.
+ *    Mac80211 will then inform the driver via %EXT_SET_KEY when a key must be
+ *    added for Rx decryption and again with %EXT_KEY_RX_TX when the driver has
+ *    to switch Tx to a new key. When the driver returns any other code than 0
+ *    for those two commands the key install is aborted and reported as failed.
+ *
+ * 2) Compatibility
+ *    This mode is for Drivers and cards which are not able to handle two
+ *    unicast key for a station on Rx, but are fine with it for Tx and can
+ *    pass trough the still encrypted MPDUs to mac80211.
+ *    The "Compatibility" Extended Key ID mode is using the key commands
+ *    %EXT_SET_KEY, %EXT_KEY_RX_TX, %EXT_DISABLE_KEY_RX and %DISABLE_KEY.
+ *    A driver setting @IEEE80211_HW_EXT_KEY_ID_COMPAT must
+ *    - implement %EXT_DISABLE_KEY_RX to switch a running key to Rx software
+ *      decryption without changing Tx handling for the key.
+ *    - Add a new key for Tx when called with %EXT_SET_KEY for the same station
+ *      with another keyid (to have a key ready allowing Tx)
+ *    - Optionally activate Rx decryption when called with %EXT_KEY_RX_TX
+ *    Only the command %EXT_KEY_RX_TX is allowed to return a value not 0, any
+ *    other command failing will abort the key install.
+ *
+ * Additionally any driver/card setting @IEEE80211_HW_EXT_KEY_ID_NATIVE or
+ * @IEEE80211_HW_EXT_KEY_ID_COMPAT must allow keyid 0 and 1 to for unicast keys.
+ */
+
 /**
  * DOC: Powersave support
  *
diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
index 334a9883894f..01849b093287 100644
--- a/net/mac80211/debugfs.c
+++ b/net/mac80211/debugfs.c
@@ -220,6 +220,7 @@ static const char *hw_flag_names[] = {
 	FLAG(STA_MMPDU_TXQ),
 	FLAG(TX_STATUS_NO_AMPDU_LEN),
 	FLAG(EXT_KEY_ID_NATIVE),
+	FLAG(EXT_KEY_ID_COMPAT),
 #undef FLAG
 };
 
diff --git a/net/mac80211/key.c b/net/mac80211/key.c
index d91503de1e1d..7f673887ec50 100644
--- a/net/mac80211/key.c
+++ b/net/mac80211/key.c
@@ -184,10 +184,32 @@ static int ieee80211_key_enable_hw_accel(struct ieee80211_key *key)
 		}
 	}
 
-	if (rx_only)
+	if (rx_only) {
+		/* EXT_KEY_ID_COMPAT drivers may scramble the payload when
+		 * using the wrong HW key for decryption. Therefore only use SW
+		 * decryption for the critical window.
+		 */
+		if (sta && pairwise && !ext_native && !local->wowlan &&
+		    sta->ptk_idx != key->conf.keyidx) {
+			struct ieee80211_key *old;
+
+			old = key_mtx_dereference(local,
+						  sta->ptk[sta->ptk_idx]);
+			if (old) {
+				if (drv_set_key(local, EXT_DISABLE_KEY_RX,
+						sdata, &sta->sta, &old->conf))
+					return -EINVAL;
+			}
+		}
+
+		/* Install the key to hardware. EXT_KEY_ID_NATIVE drivers can
+		 * use it for decryption but EXT_KEY_ID_COMPAT drivers must
+		 * prepare it as a not yet used Tx only key.
+		 */
 		cmd = EXT_SET_KEY;
-	else
+	} else {
 		cmd = SET_KEY;
+	}
 
 	ret = drv_set_key(local, cmd, sdata,
 			  sta ? &sta->sta : NULL, &key->conf);
@@ -282,6 +304,7 @@ int ieee80211_key_activate_tx(struct ieee80211_key *key)
 	struct sta_info *sta = key->sta;
 	struct ieee80211_local *local = key->local;
 	struct ieee80211_key *old;
+	bool ext_native = ieee80211_hw_check(&local->hw, EXT_KEY_ID_NATIVE);
 	int ret;
 
 	assert_key_lock(local);
@@ -294,7 +317,7 @@ int ieee80211_key_activate_tx(struct ieee80211_key *key)
 			       IEEE80211_KEY_FLAG_RESERVE_TAILROOM))
 		increment_tailroom_need_count(sdata);
 
-	if (key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) {
+	if (ext_native && key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) {
 		ret = drv_set_key(local, EXT_KEY_RX_TX, sdata,
 				  &sta->sta, &key->conf);
 		if (ret) {
@@ -309,6 +332,13 @@ int ieee80211_key_activate_tx(struct ieee80211_key *key)
 	sta->ptk_idx = key->conf.keyidx;
 	ieee80211_check_fast_xmit(sta);
 
+	if (!ext_native && key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) {
+		key->flags |= KEY_FLAG_RX_SW_CRYPTO;
+		/* Activate Rx crypto offload after max 10s when idle */
+		ieee80211_queue_delayed_work(&local->hw, &sta->ext_key_compat_wk,
+					     round_jiffies_relative(HZ * 10));
+	}
+
 	if (old) {
 		old->flags |= KEY_FLAG_RX_ONLY;
 
@@ -1109,6 +1139,37 @@ void ieee80211_free_sta_keys(struct ieee80211_local *local,
 	mutex_unlock(&local->key_mtx);
 }
 
+/* EXT_KEY_ID_COMPAT support can't install PTK keys to the card/driver for
+ * hardware decryption as long as the remote sta may use both keyids. Those
+ * cards are not aware that the keyid must be checked and try to decrypt the
+ * payload with the wrong key, which would effectively scrambling it. This
+ * worker is therefore used to activate Rx hardware decryption when we assume
+ * there will be only packets for the new key.
+ */
+void ext_key_compat_rx_offload_work(struct work_struct *wk)
+{
+	struct sta_info *sta;
+	struct ieee80211_local *local;
+	struct ieee80211_sub_if_data *sdata;
+	struct ieee80211_key *key;
+
+	sta = container_of(wk, struct sta_info, ext_key_compat_wk.work);
+	local = sta->local;
+	sdata = sta->sdata;
+
+	mutex_lock(&local->key_mtx);
+	key = key_mtx_dereference(local, sta->ptk[sta->ptk_idx]);
+
+	if (key->flags & KEY_FLAG_RX_SW_CRYPTO)
+		key->flags &= ~KEY_FLAG_RX_SW_CRYPTO;
+
+	if (drv_set_key(local, EXT_KEY_RX_TX, sdata, &sta->sta, &key->conf)) {
+		sdata_info(sdata, "Could not switch Rx to HW crypto (%d, %pM)\n",
+			   key->conf.keyidx, sta->sta.addr);
+	}
+	mutex_unlock(&local->key_mtx);
+}
+
 void ieee80211_delayed_tailroom_dec(struct work_struct *wk)
 {
 	struct ieee80211_sub_if_data *sdata;
diff --git a/net/mac80211/key.h b/net/mac80211/key.h
index 1a3da999e0c4..d74c8c36491a 100644
--- a/net/mac80211/key.h
+++ b/net/mac80211/key.h
@@ -32,12 +32,15 @@ struct sta_info;
  * @KEY_FLAG_TAINTED: Key is tainted and packets should be dropped.
  * @KEY_FLAG_CIPHER_SCHEME: This key is for a hardware cipher scheme
  * @KEY_FLAG_RX_ONLY: Pairwise key only allowed to be used on Rx.
+ * @KEY_FLAG_RX_SW_CRYPTO: This key is using Rx SW decryption to work around HW
+ *	limitations. Flag can only set when using EXT_KEY_COMPAT for max 10s.
  */
 enum ieee80211_internal_key_flags {
 	KEY_FLAG_UPLOADED_TO_HARDWARE	= BIT(0),
 	KEY_FLAG_TAINTED		= BIT(1),
 	KEY_FLAG_CIPHER_SCHEME		= BIT(2),
 	KEY_FLAG_RX_ONLY		= BIT(3),
+	KEY_FLAG_RX_SW_CRYPTO		= BIT(4),
 };
 
 enum ieee80211_internal_tkip_state {
@@ -167,5 +170,6 @@ void ieee80211_reset_crypto_tx_tailroom(struct ieee80211_sub_if_data *sdata);
 	rcu_dereference_protected(ref, lockdep_is_held(&((local)->key_mtx)))
 
 void ieee80211_delayed_tailroom_dec(struct work_struct *wk);
+void ext_key_compat_rx_offload_work(struct work_struct *wk);
 
 #endif /* IEEE80211_KEY_H */
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index ea34544985f3..dbabfa58c4c9 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -1052,7 +1052,8 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
 	}
 
 	/* mac80211 supports Extended Key ID when driver does */
-	if (ieee80211_hw_check(&local->hw, EXT_KEY_ID_NATIVE))
+	if (ieee80211_hw_check(&local->hw, EXT_KEY_ID_COMPAT) ||
+	    ieee80211_hw_check(&local->hw, EXT_KEY_ID_NATIVE))
 		wiphy_ext_feature_set(local->hw.wiphy,
 				      NL80211_EXT_FEATURE_EXT_KEY_ID);
 
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index ce786311baf4..95c13f4c7e4a 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2015,6 +2015,13 @@ ieee80211_rx_h_decrypt(struct ieee80211_rx_data *rx)
 		if (unlikely(rx->key->flags & KEY_FLAG_TAINTED))
 			return RX_DROP_MONITOR;
 
+		if (unlikely(rx->key->flags & KEY_FLAG_RX_SW_CRYPTO)) {
+			rx->key->flags &= ~KEY_FLAG_RX_SW_CRYPTO;
+			cancel_delayed_work(&rx->sta->ext_key_compat_wk);
+			ieee80211_queue_delayed_work(&rx->local->hw,
+						     &rx->sta->ext_key_compat_wk, 0);
+		}
+
 		/* TODO: add threshold stuff again */
 	} else {
 		return RX_DROP_MONITOR;
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 09c69955c6e3..a20e05439173 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -132,6 +132,7 @@ static void __cleanup_single_sta(struct sta_info *sta)
 	if (ieee80211_vif_is_mesh(&sdata->vif))
 		mesh_sta_cleanup(sta);
 
+	cancel_delayed_work_sync(&sta->ext_key_compat_wk);
 	cancel_work_sync(&sta->drv_deliver_wk);
 
 	/*
@@ -326,6 +327,8 @@ struct sta_info *sta_info_alloc(struct ieee80211_sub_if_data *sdata,
 	spin_lock_init(&sta->ps_lock);
 	INIT_WORK(&sta->drv_deliver_wk, sta_deliver_ps_frames);
 	INIT_WORK(&sta->ampdu_mlme.work, ieee80211_ba_session_work);
+	INIT_DELAYED_WORK(&sta->ext_key_compat_wk,
+			  ext_key_compat_rx_offload_work);
 	mutex_init(&sta->ampdu_mlme.mtx);
 #ifdef CONFIG_MAC80211_MESH
 	if (ieee80211_vif_is_mesh(&sdata->vif)) {
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 304a7ea24757..1fd1a349a875 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -450,6 +450,7 @@ struct ieee80211_sta_rx_stats {
  * @sdata: virtual interface this station belongs to
  * @ptk: peer keys negotiated with this station, if any
  * @ptk_idx: peer key index to use for transmissions
+ * @ext_key_compat_wk: supports PTK key installs when using EXT_KEY_ID_COMPAT
  * @gtk: group keys negotiated with this station, if any
  * @rate_ctrl: rate control algorithm reference
  * @rate_ctrl_lock: spinlock used to protect rate control data
@@ -530,6 +531,7 @@ struct sta_info {
 	struct ieee80211_sub_if_data *sdata;
 	struct ieee80211_key __rcu *gtk[NUM_DEFAULT_KEYS + NUM_DEFAULT_MGMT_KEYS];
 	struct ieee80211_key __rcu *ptk[NUM_DEFAULT_KEYS];
+	struct delayed_work ext_key_compat_wk;
 	u8 ptk_idx;
 	struct rate_control_ref *rate_ctrl;
 	void *rate_ctrl_priv;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [RFC PATCH v3 05/12] mac80211: Mark A-MPDU keyid borders for drivers
  2019-02-10 21:06 [RFC PATCH v3 00/12] Draft for Extended Key ID support Alexander Wetzel
                   ` (3 preceding siblings ...)
  2019-02-10 21:06 ` [RFC PATCH v3 04/12] mac80211: Compatibility " Alexander Wetzel
@ 2019-02-10 21:06 ` Alexander Wetzel
  2019-02-15 11:50   ` Johannes Berg
  2019-02-10 21:06 ` [RFC PATCH v3 06/12] mac80211_hwsim: Ext Key ID support (NATIVE) Alexander Wetzel
                   ` (7 subsequent siblings)
  12 siblings, 1 reply; 23+ messages in thread
From: Alexander Wetzel @ 2019-02-10 21:06 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Alexander Wetzel

IEEE 802.11-2016 "9.7.3 A-MPDU contents" forbids aggregating MPDUs with
different keyids.
Extended Key ID support breaks the assumption that all unicast MPDUs for
one station can always be aggregated.

Inform the drivers about a keyid border by dropping the
@IEEE80211_TX_CTL_AMPDU flag for the last MPDU using the current keyid.

Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de>
---

This patch here is totally unnecessary when we decide that IEEE 802.11 -
2016 is not meaning what is referenced in the commit message above:-)
The exact wording in the standard is:
"All protected MPDUs within an A-MPDU have the same Key ID."

The intent of the wording was probably written without considering
Extended Key IDs. At least it makes no sense for me to forbid mixing
MPDUs using keyid 0 and 1 in one A-MPDU. Nevertheless it says what it
says and there may now be cards/drivers depending on that and e.g. only
check it for the first MPDU. The lost MPDUs would then be our fault, for
aggregating together what according to the standard must not. 

But even with that view we still don't need the patch:
A-MPDU aggregation is the job for the driver. We simply can offload the
problem to the drivers.

On the other side mac80211 is in what I consider a better position to
determine and mark the MPDU keyid border, saving the driver the effort
to parse the MPDUs again and keep track of the "current" keyid. It also
allows the driver to terminate a running A-MPDU frame when it gets the
last packet for the old key instead one frame later.

To get around what looked like a nightmare of synchronisation problems
this patch puts the Tx key switch into the Tx path. Handling idle
connections when we don't want to accept that the first packet of a
rekey may still use the key-before-current makes it more complex. 

The code is assuming that the driver is not aggregating MPDUs more than
5s apart. We probably don't have wait nearly so long but I'm not sure
what is the minimum time.

The patch also brought up a interesting problem: We are out of sta_info
flags and skb CB also has no room left to add new ones. I worked around
that by redefining how IEEE80211_TX_CTL_AMPDU has to be handled for
Extended Key ID A-MPDU sessions. If you think that puts too much stain
on the API I would need a pointer what would be considered an acceptable
solution to the problem.

But I also fully understand when you think this patch goes too far and
want it thrown out and want it handled somehow else:-).


 net/mac80211/key.c      | 10 +++++--
 net/mac80211/key.h      |  1 +
 net/mac80211/sta_info.c |  1 +
 net/mac80211/sta_info.h |  2 ++
 net/mac80211/tx.c       | 64 +++++++++++++++++++++++++++++++++++------
 5 files changed, 68 insertions(+), 10 deletions(-)

diff --git a/net/mac80211/key.c b/net/mac80211/key.c
index 7f673887ec50..dee18f61fe33 100644
--- a/net/mac80211/key.c
+++ b/net/mac80211/key.c
@@ -309,6 +309,10 @@ int ieee80211_key_activate_tx(struct ieee80211_key *key)
 
 	assert_key_lock(local);
 
+	/* Two key activations must not overlap */
+	if (WARN_ON(sta->ptk_idx_next != INVALID_PTK_KEYIDX))
+		return -EOPNOTSUPP;
+
 	key->flags &= ~KEY_FLAG_RX_ONLY;
 
 	if (!(key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) ||
@@ -329,8 +333,9 @@ int ieee80211_key_activate_tx(struct ieee80211_key *key)
 	}
 
 	old = key_mtx_dereference(local, sta->ptk[sta->ptk_idx]);
-	sta->ptk_idx = key->conf.keyidx;
-	ieee80211_check_fast_xmit(sta);
+
+	sta->ptk_idx_next = key->conf.keyidx;
+	key->force_use_after = jiffies + 5 * HZ;
 
 	if (!ext_native && key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) {
 		key->flags |= KEY_FLAG_RX_SW_CRYPTO;
@@ -577,6 +582,7 @@ ieee80211_key_alloc(u32 cipher, int idx, size_t key_len,
 	 */
 	key->conf.flags = 0;
 	key->flags = 0;
+	key->force_use_after = 0;
 
 	key->conf.cipher = cipher;
 	key->conf.keyidx = idx;
diff --git a/net/mac80211/key.h b/net/mac80211/key.h
index d74c8c36491a..48975d56e792 100644
--- a/net/mac80211/key.h
+++ b/net/mac80211/key.h
@@ -65,6 +65,7 @@ struct ieee80211_key {
 	struct ieee80211_local *local;
 	struct ieee80211_sub_if_data *sdata;
 	struct sta_info *sta;
+	unsigned long force_use_after;
 
 	/* for sdata list */
 	struct list_head list;
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index a20e05439173..6fe40844f485 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -358,6 +358,7 @@ struct sta_info *sta_info_alloc(struct ieee80211_sub_if_data *sdata,
 	 */
 	BUILD_BUG_ON(ARRAY_SIZE(sta->ptk) <= INVALID_PTK_KEYIDX);
 	sta->ptk_idx = INVALID_PTK_KEYIDX;
+	sta->ptk_idx_next = INVALID_PTK_KEYIDX;
 
 	sta->local = local;
 	sta->sdata = sdata;
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 1fd1a349a875..6eff946bc55a 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -450,6 +450,7 @@ struct ieee80211_sta_rx_stats {
  * @sdata: virtual interface this station belongs to
  * @ptk: peer keys negotiated with this station, if any
  * @ptk_idx: peer key index to use for transmissions
+ * @ptk_idx_next: peer key index in activation (Extended Key ID only)
  * @ext_key_compat_wk: supports PTK key installs when using EXT_KEY_ID_COMPAT
  * @gtk: group keys negotiated with this station, if any
  * @rate_ctrl: rate control algorithm reference
@@ -533,6 +534,7 @@ struct sta_info {
 	struct ieee80211_key __rcu *ptk[NUM_DEFAULT_KEYS];
 	struct delayed_work ext_key_compat_wk;
 	u8 ptk_idx;
+	u8 ptk_idx_next;
 	struct rate_control_ref *rate_ctrl;
 	void *rate_ctrl_priv;
 	spinlock_t rate_ctrl_lock;
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 111bd6c490a6..d3825eca9e64 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -586,6 +586,51 @@ ieee80211_tx_h_check_control_port_protocol(struct ieee80211_tx_data *tx)
 	return TX_CONTINUE;
 }
 
+static struct ieee80211_key debug_noinline
+*ieee80211_select_sta_key(struct ieee80211_tx_data *tx)
+{
+	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)tx->skb->data;
+	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx->skb);
+	struct sta_info *sta = tx->sta;
+	struct ieee80211_key *key;
+	struct ieee80211_key *next_key;
+
+	key = rcu_dereference(tx->sta->ptk[tx->sta->ptk_idx]);
+
+	if (likely(sta->ptk_idx_next == INVALID_PTK_KEYIDX))
+		return key;
+
+	/* Only when using Extended Key ID the code below can be executed */
+
+	if (!ieee80211_is_data_present(hdr->frame_control))
+		return key;
+
+	if (sta->ptk_idx_next == sta->ptk_idx) {
+		/* First packet using new key with A-MPDU active*/
+		sta->ptk_idx_next = INVALID_PTK_KEYIDX;
+		ieee80211_check_fast_xmit(tx->sta);
+		return key;
+	}
+
+	next_key = rcu_dereference(sta->ptk[sta->ptk_idx_next]);
+	if (!key || !(info->flags & IEEE80211_TX_CTL_AMPDU) ||
+	    (next_key->force_use_after &&
+	     time_is_before_jiffies(next_key->force_use_after))) {
+		/* nothing special to do, just start using the new key */
+		sta->ptk_idx = sta->ptk_idx_next;
+		sta->ptk_idx_next = INVALID_PTK_KEYIDX;
+		next_key->force_use_after = 0;
+		ieee80211_check_fast_xmit(tx->sta);
+		return next_key;
+	}
+
+	/* Last packet with old key with A-MPDU active */
+	sta->ptk_idx = sta->ptk_idx_next;
+	next_key->force_use_after = 0;
+	info->flags &= ~IEEE80211_TX_CTL_AMPDU;
+	return key;
+}
+
 static ieee80211_tx_result debug_noinline
 ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
 {
@@ -595,9 +640,8 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
 
 	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])))
-		tx->key = key;
+	else if (tx->sta)
+		tx->key = ieee80211_select_sta_key(tx);
 	else if (ieee80211_is_group_privacy_action(tx->skb) &&
 		(key = rcu_dereference(tx->sdata->default_multicast_key)))
 		tx->key = key;
@@ -3414,6 +3458,10 @@ static bool ieee80211_xmit_fast(struct ieee80211_sub_if_data *sdata,
 	if (skb->sk && skb_shinfo(skb)->tx_flags & SKBTX_WIFI_STATUS)
 		return false;
 
+	/* ieee80211_key_activate_tx() requests to change key */
+	if (unlikely(sta->ptk_idx_next != INVALID_PTK_KEYIDX))
+		return false;
+
 	if (hdr->frame_control & cpu_to_le16(IEEE80211_STYPE_QOS_DATA)) {
 		tid = skb->priority & IEEE80211_QOS_CTL_TAG1D_MASK;
 		tid_tx = rcu_dereference(sta->ampdu_mlme.tid_tx[tid]);
@@ -3556,6 +3604,11 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
 	if (txq->sta)
 		tx.sta = container_of(txq->sta, struct sta_info, sta);
 
+	if (test_bit(IEEE80211_TXQ_AMPDU, &txqi->flags))
+		info->flags |= IEEE80211_TX_CTL_AMPDU;
+	else
+		info->flags &= ~IEEE80211_TX_CTL_AMPDU;
+
 	/*
 	 * The key can be removed while the packet was queued, so need to call
 	 * this here to get the current key.
@@ -3566,11 +3619,6 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
 		goto begin;
 	}
 
-	if (test_bit(IEEE80211_TXQ_AMPDU, &txqi->flags))
-		info->flags |= IEEE80211_TX_CTL_AMPDU;
-	else
-		info->flags &= ~IEEE80211_TX_CTL_AMPDU;
-
 	if (info->control.flags & IEEE80211_TX_CTRL_FAST_XMIT) {
 		struct sta_info *sta = container_of(txq->sta, struct sta_info,
 						    sta);
-- 
2.20.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [RFC PATCH v3 06/12] mac80211_hwsim: Ext Key ID support (NATIVE)
  2019-02-10 21:06 [RFC PATCH v3 00/12] Draft for Extended Key ID support Alexander Wetzel
                   ` (4 preceding siblings ...)
  2019-02-10 21:06 ` [RFC PATCH v3 05/12] mac80211: Mark A-MPDU keyid borders for drivers Alexander Wetzel
@ 2019-02-10 21:06 ` Alexander Wetzel
  2019-02-10 21:06 ` [RFC PATCH v3 07/12] iwlwifi: Extended " Alexander Wetzel
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 23+ messages in thread
From: Alexander Wetzel @ 2019-02-10 21:06 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Alexander Wetzel

Driver is not supporting hardware encryption and therefore fully
compatible with Extended Key ID.

Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de>
---
 drivers/net/wireless/mac80211_hwsim.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 87be2b18063a..306583316f5d 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -2799,6 +2799,7 @@ static int mac80211_hwsim_new_radio(struct genl_info *info,
 	ieee80211_hw_set(hw, SIGNAL_DBM);
 	ieee80211_hw_set(hw, SUPPORTS_PS);
 	ieee80211_hw_set(hw, TDLS_WIDER_BW);
+	ieee80211_hw_set(hw, EXT_KEY_ID_NATIVE);
 	if (rctbl)
 		ieee80211_hw_set(hw, SUPPORTS_RC_TABLE);
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [RFC PATCH v3 07/12] iwlwifi: Extended Key ID support (NATIVE)
  2019-02-10 21:06 [RFC PATCH v3 00/12] Draft for Extended Key ID support Alexander Wetzel
                   ` (5 preceding siblings ...)
  2019-02-10 21:06 ` [RFC PATCH v3 06/12] mac80211_hwsim: Ext Key ID support (NATIVE) Alexander Wetzel
@ 2019-02-10 21:06 ` " Alexander Wetzel
  2019-02-15 11:52   ` Johannes Berg
  2019-02-10 21:06 ` [RFC PATCH v3 08/12] iwlwifi: dvm - EXT_KEY_ID A-MPDU API update Alexander Wetzel
                   ` (5 subsequent siblings)
  12 siblings, 1 reply; 23+ messages in thread
From: Alexander Wetzel @ 2019-02-10 21:06 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Alexander Wetzel

This is not ready for merge and has known issues.
The patch is only for discussions to sort out how to handle it correctly!

Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de>
---

iwlwifi intel cards had two big surprises:

Assuming I did not make some stupid errors it looks like my old
"Intel Corporation Centrino Ultimate-N 6300 (rev 3e)" using ucode 
9.221.4.1 build 25532 is perfectly fine with two keys uploaded to
harware and honoring the keyid in the MPDUs. For a card launched 2011
that's a pleasant surprise:-)

A much shorter test with a modern "Intel Corporation Wireless 8265 /
8175 (rev 78)" using ucode version 36.e91976c0.0 shows what seems to be
MPDUs decoded with the wrong key at each rekey and therefore a candidate
for the COMPAT support only..
So the bad news seems to be, that the modern card dropped the support.

It also seems to force us to add some per-card or per-firmware depending
check to decide which card can use the Native Extended Key ID support
and use the Compat mode for the rest.
Is there any way to find out which cards/firmware can be used with
Extended Key ID?
I also have tested patch for iwldvm using the Compat mode and I think
mvm cards will also work with that.

 drivers/net/wireless/intel/iwlwifi/dvm/mac80211.c | 5 +++++
 drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/drivers/net/wireless/intel/iwlwifi/dvm/mac80211.c b/drivers/net/wireless/intel/iwlwifi/dvm/mac80211.c
index 54b759cec8b3..0dd5c19ac412 100644
--- a/drivers/net/wireless/intel/iwlwifi/dvm/mac80211.c
+++ b/drivers/net/wireless/intel/iwlwifi/dvm/mac80211.c
@@ -111,6 +111,7 @@ int iwlagn_mac_setup_register(struct iwl_priv *priv,
 	ieee80211_hw_set(hw, SUPPORTS_DYNAMIC_PS);
 	ieee80211_hw_set(hw, SUPPORT_FAST_XMIT);
 	ieee80211_hw_set(hw, WANT_MONITOR_VIF);
+	ieee80211_hw_set(hw, EXT_KEY_ID_NATIVE);
 
 	if (priv->trans->max_skb_frags)
 		hw->netdev_features = NETIF_F_HIGHDMA | NETIF_F_SG;
@@ -676,6 +677,7 @@ static int iwlagn_mac_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 
 
 	switch (cmd) {
+	case EXT_SET_KEY:
 	case SET_KEY:
 		if (is_default_wep_key) {
 			ret = iwl_set_default_wep_key(priv, vif_priv->ctx, key);
@@ -701,6 +703,9 @@ static int iwlagn_mac_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 
 		IWL_DEBUG_MAC80211(priv, "disable hwcrypto key\n");
 		break;
+	case EXT_KEY_RX_TX:
+		ret = 0;
+		break;
 	default:
 		ret = -EINVAL;
 	}
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
index fc251cc47b7f..102367d2572f 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
@@ -442,6 +442,7 @@ int iwl_mvm_mac_setup_register(struct iwl_mvm *mvm)
 	ieee80211_hw_set(hw, STA_MMPDU_TXQ);
 	ieee80211_hw_set(hw, TX_AMSDU);
 	ieee80211_hw_set(hw, TX_FRAG_LIST);
+	ieee80211_hw_set(hw, EXT_KEY_ID_NATIVE);
 
 	if (iwl_mvm_has_tlc_offload(mvm)) {
 		ieee80211_hw_set(hw, TX_AMPDU_SETUP_IN_HW);
@@ -3357,6 +3358,7 @@ static int iwl_mvm_mac_set_key(struct ieee80211_hw *hw,
 	mutex_lock(&mvm->mutex);
 
 	switch (cmd) {
+	case EXT_SET_KEY:
 	case SET_KEY:
 		if ((vif->type == NL80211_IFTYPE_ADHOC ||
 		     vif->type == NL80211_IFTYPE_AP) && !sta) {
@@ -3464,6 +3466,9 @@ static int iwl_mvm_mac_set_key(struct ieee80211_hw *hw,
 		IWL_DEBUG_MAC80211(mvm, "disable hwcrypto key\n");
 		ret = iwl_mvm_remove_sta_key(mvm, vif, sta, key);
 		break;
+	case EXT_KEY_RX_TX:
+		ret = 0;
+		break;
 	default:
 		ret = -EINVAL;
 	}
-- 
2.20.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [RFC PATCH v3 08/12] iwlwifi: dvm - EXT_KEY_ID A-MPDU API update
  2019-02-10 21:06 [RFC PATCH v3 00/12] Draft for Extended Key ID support Alexander Wetzel
                   ` (6 preceding siblings ...)
  2019-02-10 21:06 ` [RFC PATCH v3 07/12] iwlwifi: Extended " Alexander Wetzel
@ 2019-02-10 21:06 ` Alexander Wetzel
  2019-02-15 11:54   ` Johannes Berg
  2019-02-10 21:06 ` [RFC PATCH v3 09/12] ath: Basic Extended Key ID support (COMPAT+NATIVE) Alexander Wetzel
                   ` (4 subsequent siblings)
  12 siblings, 1 reply; 23+ messages in thread
From: Alexander Wetzel @ 2019-02-10 21:06 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Alexander Wetzel

When using Extended Key ID mac80211 drops @IEEE80211_TX_CTL_AMPDU for
the last packet which can be added to a A-MPDU in preparation.

Don't throw a warning and just handle the frame as if
@IEEE80211_TX_CTL_AMPDU would have been set.

Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de>
---

I cold not figure out so far how to make sure the card will not mix
A-MPDU frames. Looks like that is handled fully in the HW and I didn't
find any interface to influence it. (It even may work already, I have
some problems to capture A-MPDU frames with A-MPDU information intact
and the topic was pretty low on the list so far. After all it works...)

This patch is therefore basically just using aggregation when it's
enabled and ignores the key border signal from mac80211.

 drivers/net/wireless/intel/iwlwifi/dvm/tx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/dvm/tx.c b/drivers/net/wireless/intel/iwlwifi/dvm/tx.c
index 4ff323a3a4e5..478f8e1c3e52 100644
--- a/drivers/net/wireless/intel/iwlwifi/dvm/tx.c
+++ b/drivers/net/wireless/intel/iwlwifi/dvm/tx.c
@@ -420,7 +420,7 @@ int iwlagn_tx_skb(struct iwl_priv *priv,
 		hdr->seq_ctrl |= cpu_to_le16(seq_number);
 		seq_number += 0x10;
 
-		if (info->flags & IEEE80211_TX_CTL_AMPDU)
+		if (tid_data->agg.state == IWL_AGG_ON)
 			is_agg = true;
 		is_data_qos = true;
 	}
-- 
2.20.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [RFC PATCH v3 09/12] ath: Basic Extended Key ID support (COMPAT+NATIVE)
  2019-02-10 21:06 [RFC PATCH v3 00/12] Draft for Extended Key ID support Alexander Wetzel
                   ` (7 preceding siblings ...)
  2019-02-10 21:06 ` [RFC PATCH v3 08/12] iwlwifi: dvm - EXT_KEY_ID A-MPDU API update Alexander Wetzel
@ 2019-02-10 21:06 ` Alexander Wetzel
  2019-02-13 11:05   ` Kalle Valo
  2019-02-10 21:06 ` [RFC PATCH v3 10/12] ath5k: ath_key_config() API compatibility update Alexander Wetzel
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 23+ messages in thread
From: Alexander Wetzel @ 2019-02-10 21:06 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Alexander Wetzel

Extend the shared ath key cache code to support Extended Key ID.

The key cache code has to accept unicast keys to use key idx 1 and allow
drivers to enable/disable hardware Rx decryption for a key independent
from Tx.

Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de>
---

I know this is the wrong audience to discuss ath drivers. It's only
included here as an example and POC that the Compatibility Extended Key
ID means for drivers.
This has so far only got the minimal attention needed to get it working
for my AP used for tests. The idea is, to discuss that with the proper
audience once we know what mac80211 Extended Key ID support will look
like.

 drivers/net/wireless/ath/ath.h |  7 ++++++-
 drivers/net/wireless/ath/key.c | 35 +++++++++++++++++++++++++++++-----
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
index cc45ccfea5af..465629448fdf 100644
--- a/drivers/net/wireless/ath/ath.h
+++ b/drivers/net/wireless/ath/ath.h
@@ -202,8 +202,13 @@ void ath_key_delete(struct ath_common *common, struct ieee80211_key_conf *key);
 int ath_key_config(struct ath_common *common,
 			  struct ieee80211_vif *vif,
 			  struct ieee80211_sta *sta,
-			  struct ieee80211_key_conf *key);
+			  struct ieee80211_key_conf *key,
+			  bool rx_accel);
 bool ath_hw_keyreset(struct ath_common *common, u16 entry);
+bool ath_hw_rx_crypt(struct ath_common *common,
+		     struct ieee80211_key_conf *key,
+		     struct ieee80211_sta *sta,
+		     bool rx_accel);
 void ath_hw_cycle_counters_update(struct ath_common *common);
 int32_t ath_hw_get_listen_time(struct ath_common *common);
 
diff --git a/drivers/net/wireless/ath/key.c b/drivers/net/wireless/ath/key.c
index 689fab9acf10..ced1c89102ad 100644
--- a/drivers/net/wireless/ath/key.c
+++ b/drivers/net/wireless/ath/key.c
@@ -126,6 +126,23 @@ static bool ath_hw_keysetmac(struct ath_common *common,
 	return true;
 }
 
+bool ath_hw_rx_crypt(struct ath_common *common,
+		     struct ieee80211_key_conf *key,
+		     struct ieee80211_sta *sta,
+		     bool rx_accel)
+{
+	const u8 *mac = NULL;
+
+	if (!sta || !test_bit(key->hw_key_idx, common->keymap))
+		return false;
+
+	if (rx_accel)
+		mac = sta->addr;
+
+	return ath_hw_keysetmac(common, key->hw_key_idx, mac);
+}
+EXPORT_SYMBOL(ath_hw_rx_crypt);
+
 static bool ath_hw_set_keycache_entry(struct ath_common *common, u16 entry,
 				      const struct ath_keyval *k,
 				      const u8 *mac)
@@ -473,7 +490,8 @@ static int ath_reserve_key_cache_slot(struct ath_common *common,
 int ath_key_config(struct ath_common *common,
 			  struct ieee80211_vif *vif,
 			  struct ieee80211_sta *sta,
-			  struct ieee80211_key_conf *key)
+			  struct ieee80211_key_conf *key,
+			  bool rx_accel)
 {
 	struct ath_keyval hk;
 	const u8 *mac = NULL;
@@ -527,21 +545,28 @@ int ath_key_config(struct ath_common *common,
 			idx = key->keyidx;
 			break;
 		}
-	} else if (key->keyidx) {
+	} else if (key->keyidx > 1) {
 		if (WARN_ON(!sta))
 			return -EOPNOTSUPP;
 		mac = sta->addr;
 
 		if (vif->type != NL80211_IFTYPE_AP) {
-			/* Only keyidx 0 should be used with unicast key, but
-			 * allow this for client mode for now. */
+			/* Only keyidx 0 and when using Extended Key ID 1 should
+			 * be used with a unicast key. But allow this for client
+			 * mode for now.
+			 */
 			idx = key->keyidx;
 		} else
 			return -EIO;
 	} else {
 		if (WARN_ON(!sta))
 			return -EOPNOTSUPP;
-		mac = sta->addr;
+
+		/* Handle sta Tx only keys like GTK keys for now */
+		if (rx_accel)
+			mac = sta->addr;
+		else
+			mac = NULL;
 
 		idx = ath_reserve_key_cache_slot(common, key->cipher);
 	}
-- 
2.20.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [RFC PATCH v3 10/12] ath5k: ath_key_config() API compatibility update
  2019-02-10 21:06 [RFC PATCH v3 00/12] Draft for Extended Key ID support Alexander Wetzel
                   ` (8 preceding siblings ...)
  2019-02-10 21:06 ` [RFC PATCH v3 09/12] ath: Basic Extended Key ID support (COMPAT+NATIVE) Alexander Wetzel
@ 2019-02-10 21:06 ` Alexander Wetzel
  2019-02-10 21:06 ` [RFC PATCH v3 11/12] ath9k: Extended Key ID support (COMPAT) Alexander Wetzel
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 23+ messages in thread
From: Alexander Wetzel @ 2019-02-10 21:06 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Alexander Wetzel

Update ath_key_config() call to work with Extended Key ID update for ath.

Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de>
---

This patch is only provided to not break compile for ath5k. 
(ath5k is very likely also able to support Extended Key ID is
Compatibility mode.)

 drivers/net/wireless/ath/ath5k/mac80211-ops.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath5k/mac80211-ops.c b/drivers/net/wireless/ath/ath5k/mac80211-ops.c
index 16e052d02c94..cbabb784decb 100644
--- a/drivers/net/wireless/ath/ath5k/mac80211-ops.c
+++ b/drivers/net/wireless/ath/ath5k/mac80211-ops.c
@@ -509,7 +509,7 @@ ath5k_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 
 	switch (cmd) {
 	case SET_KEY:
-		ret = ath_key_config(common, vif, sta, key);
+		ret = ath_key_config(common, vif, sta, key, true);
 		if (ret >= 0) {
 			key->hw_key_idx = ret;
 			/* push IV and Michael MIC generation to stack */
-- 
2.20.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [RFC PATCH v3 11/12] ath9k: Extended Key ID support (COMPAT)
  2019-02-10 21:06 [RFC PATCH v3 00/12] Draft for Extended Key ID support Alexander Wetzel
                   ` (9 preceding siblings ...)
  2019-02-10 21:06 ` [RFC PATCH v3 10/12] ath5k: ath_key_config() API compatibility update Alexander Wetzel
@ 2019-02-10 21:06 ` Alexander Wetzel
  2019-02-10 21:06 ` [RFC PATCH v3 12/12] ath9k: EXT_KEY_ID A-MPDU API update Alexander Wetzel
  2019-02-15 11:10 ` [RFC PATCH v3 00/12] Draft for Extended Key ID support Johannes Berg
  12 siblings, 0 replies; 23+ messages in thread
From: Alexander Wetzel @ 2019-02-10 21:06 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Alexander Wetzel

Implements %EXT_SET_KEY, %EXT_KEY_RX_TX and %EXT_DISABLE_KEY_RX and
enables EXT_KEY_ID_COMPAT.

Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de>
---

Like the generic ath patch to provide Extended Key ID support just the
minimum needed to get it working in AP mode and serve as an reference
how Compatibility Extended Key ID support looks like from a driver
perspective.

 drivers/net/wireless/ath/ath9k/htc_drv_main.c |  2 +-
 drivers/net/wireless/ath/ath9k/init.c         |  1 +
 drivers/net/wireless/ath/ath9k/main.c         | 20 ++++++++++++++++---
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_main.c b/drivers/net/wireless/ath/ath9k/htc_drv_main.c
index a82ad739ab80..2708572616f2 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_main.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_main.c
@@ -1446,7 +1446,7 @@ static int ath9k_htc_set_key(struct ieee80211_hw *hw,
 
 	switch (cmd) {
 	case SET_KEY:
-		ret = ath_key_config(common, vif, sta, key);
+		ret = ath_key_config(common, vif, sta, key, true);
 		if (ret >= 0) {
 			key->hw_key_idx = ret;
 			/* push IV and Michael MIC generation to stack */
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index c070a9e51ebf..ac1c6d59b954 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -929,6 +929,7 @@ static void ath9k_set_hw_capab(struct ath_softc *sc, struct ieee80211_hw *hw)
 	ieee80211_hw_set(hw, HOST_BROADCAST_PS_BUFFERING);
 	ieee80211_hw_set(hw, SUPPORT_FAST_XMIT);
 	ieee80211_hw_set(hw, SUPPORTS_CLONED_SKBS);
+	ieee80211_hw_set(hw, EXT_KEY_ID_COMPAT);
 
 	if (ath9k_ps_enable)
 		ieee80211_hw_set(hw, SUPPORTS_PS);
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index f23cb2f3d296..880687f09157 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1518,7 +1518,7 @@ static int ath9k_sta_add(struct ieee80211_hw *hw,
 	    vif->type != NL80211_IFTYPE_AP_VLAN)
 		return 0;
 
-	key = ath_key_config(common, vif, sta, &ps_key);
+	key = ath_key_config(common, vif, sta, &ps_key, true);
 	if (key > 0) {
 		an->ps_key = key;
 		an->key_idx[0] = key;
@@ -1675,9 +1675,13 @@ static int ath9k_set_key(struct ieee80211_hw *hw,
 	struct ath_common *common = ath9k_hw_common(sc->sc_ah);
 	struct ath_node *an = NULL;
 	int ret = 0, i;
+	bool rx_accel = true;
 
-	if (ath9k_modparam_nohwcrypt)
+	if (ath9k_modparam_nohwcrypt) {
+		if (cmd == EXT_DISABLE_KEY_RX || cmd == EXT_KEY_RX_TX)
+			return 0;
 		return -ENOSPC;
+	}
 
 	if ((vif->type == NL80211_IFTYPE_ADHOC ||
 	     vif->type == NL80211_IFTYPE_MESH_POINT) &&
@@ -1701,12 +1705,15 @@ static int ath9k_set_key(struct ieee80211_hw *hw,
 		an = (struct ath_node *)sta->drv_priv;
 
 	switch (cmd) {
+	case EXT_SET_KEY:
+		rx_accel = false;
+		/* Fall trough */
 	case SET_KEY:
 		if (sta)
 			ath9k_del_ps_key(sc, vif, sta);
 
 		key->hw_key_idx = 0;
-		ret = ath_key_config(common, vif, sta, key);
+		ret = ath_key_config(common, vif, sta, key, rx_accel);
 		if (ret >= 0) {
 			key->hw_key_idx = ret;
 			/* push IV and Michael MIC generation to stack */
@@ -1740,6 +1747,13 @@ static int ath9k_set_key(struct ieee80211_hw *hw,
 		}
 		key->hw_key_idx = 0;
 		break;
+	case EXT_DISABLE_KEY_RX:
+		rx_accel = false;
+		/* fall trough */
+	case EXT_KEY_RX_TX:
+		if (ath_hw_rx_crypt(common, key, sta, rx_accel))
+			ret = 0;
+		break;
 	default:
 		ret = -EINVAL;
 	}
-- 
2.20.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [RFC PATCH v3 12/12] ath9k: EXT_KEY_ID A-MPDU API update
  2019-02-10 21:06 [RFC PATCH v3 00/12] Draft for Extended Key ID support Alexander Wetzel
                   ` (10 preceding siblings ...)
  2019-02-10 21:06 ` [RFC PATCH v3 11/12] ath9k: Extended Key ID support (COMPAT) Alexander Wetzel
@ 2019-02-10 21:06 ` Alexander Wetzel
  2019-02-15 11:10 ` [RFC PATCH v3 00/12] Draft for Extended Key ID support Johannes Berg
  12 siblings, 0 replies; 23+ messages in thread
From: Alexander Wetzel @ 2019-02-10 21:06 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Alexander Wetzel

When using Extended Key ID mac80211 drops @IEEE80211_TX_CTL_AMPDU for
the last packet which can be added to a A-MPDU in preparation.

Finalize A-MPDU immediately and start a new aggregation.

Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de>
---

This is the (b)leading edge of my current attempt to figure out a
solution to the "must not aggregate keyid 0 and 1 frames together"
problem.

I've not even tested that patch properly, yet..
So it only shows how I think ath9k driver can use the A-MPDU border
signal to send out the A-MPDU frame currently under construction.

May be useful to figure out if we want to implement A-MPDU border signal
in mac80211, through.

 drivers/net/wireless/ath/ath9k/xmit.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index a24265018cb2..84bbce6f212f 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -990,10 +990,8 @@ ath_tx_get_tid_subframe(struct ath_softc *sc, struct ath_txq *txq,
 		 * from a previous session or a failed attempt in the queue.
 		 * Send them out as normal data frames
 		 */
-		if (!tid->active)
+		if (!tid->active) {
 			tx_info->flags &= ~IEEE80211_TX_CTL_AMPDU;
-
-		if (!(tx_info->flags & IEEE80211_TX_CTL_AMPDU)) {
 			bf->bf_state.bf_type = 0;
 			return bf;
 		}
@@ -1017,12 +1015,18 @@ ath_tx_get_tid_subframe(struct ath_softc *sc, struct ath_txq *txq,
 			break;
 		}
 
-		if (tid->bar_index > ATH_BA_INDEX(tid->seq_start, seqno)) {
+		if (tid->bar_index > ATH_BA_INDEX(tid->seq_start, seqno) ||
+		    !(tx_info->flags & IEEE80211_TX_CTL_AMPDU)) {
 			struct ath_tx_status ts = {};
 			struct list_head bf_head;
 
 			INIT_LIST_HEAD(&bf_head);
-			list_add(&bf->list, &bf_head);
+			if (tx_info->flags & IEEE80211_TX_CTL_AMPDU &&
+			    tid->bar_index <= ATH_BA_INDEX(tid->seq_start, seqno))
+				list_add(&bf->list, &bf_head);
+			else
+				ath_tx_addto_baw(sc, tid, bf);
+
 			ath_tx_update_baw(sc, tid, bf);
 			ath_tx_complete_buf(sc, bf, txq, &bf_head, NULL, &ts, 0);
 			continue;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC PATCH v3 09/12] ath: Basic Extended Key ID support (COMPAT+NATIVE)
  2019-02-10 21:06 ` [RFC PATCH v3 09/12] ath: Basic Extended Key ID support (COMPAT+NATIVE) Alexander Wetzel
@ 2019-02-13 11:05   ` Kalle Valo
  2019-02-13 23:15     ` Alexander Wetzel
  0 siblings, 1 reply; 23+ messages in thread
From: Kalle Valo @ 2019-02-13 11:05 UTC (permalink / raw)
  To: Alexander Wetzel; +Cc: johannes, linux-wireless

Alexander Wetzel <alexander@wetzel-home.de> writes:

> Extend the shared ath key cache code to support Extended Key ID.
>
> The key cache code has to accept unicast keys to use key idx 1 and allow
> drivers to enable/disable hardware Rx decryption for a key independent
> from Tx.
>
> Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de>
> ---
>
> I know this is the wrong audience to discuss ath drivers.

I think this is the right forum. Do note that somewhere in this patch
(in the cover letter) you mentioned "all ath drivers" but AFAICS this
patch only changes functionality for ath5k, ath9k and ath9k_htc. All the
rest like wil6210, ath6kl and ath10k are unaffected.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC PATCH v3 09/12] ath: Basic Extended Key ID support (COMPAT+NATIVE)
  2019-02-13 11:05   ` Kalle Valo
@ 2019-02-13 23:15     ` Alexander Wetzel
  0 siblings, 0 replies; 23+ messages in thread
From: Alexander Wetzel @ 2019-02-13 23:15 UTC (permalink / raw)
  To: Kalle Valo; +Cc: johannes, linux-wireless

> 
>> Extend the shared ath key cache code to support Extended Key ID.
>>
>> The key cache code has to accept unicast keys to use key idx 1 and allow
>> drivers to enable/disable hardware Rx decryption for a key independent
>> from Tx.
>>
>> Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de>
>> ---
>>
>> I know this is the wrong audience to discuss ath drivers.
> 
> I think this is the right forum. Do note that somewhere in this patch
You are of course right. I mixed that up somehow.

We can of course also discuss the ath patches any time :-)
My initial plan was, to get the nl80211/mac80211 API finalized and then 
get them reviewed together with another planned fix after some more 
polishing.

At this stage they are just a POC and not ready for merge. They work 
with ath9k in AP (vlan) mode and I believe managed mode should either 
work or need some trivial fix only. (There even seems to be a chance 
that managed mode could allow the usage of the NATIVE Extended Key ID 
mode, but so far I could not tested that.)

> (in the cover letter) you mentioned "all ath drivers" but AFAICS this
> patch only changes functionality for ath5k, ath9k and ath9k_htc. All the
> rest like wil6210, ath6kl and ath10k are unaffected.
> 
You are right, I should have used "shared ath key cache code" in the 
Cover Letter, as in the patch itself. This is not (yet) an attempt to 
implement Extended Key ID for anything else than ath9k AP mode. So any 
driver not using ath_key_config() won't be affected at all.

Now I believe it's possible for all Atheros drivers but the ath10k to 
get support. As long as a card can work with SW crypto we only need a 
way to disable Rx HW crypto for a running key without impact for ongoing Tx.
But the initial results when trying my hand at ath10k are strongly 
indicating the best we can hope there is SW encryption only with CT 
firmware... or maybe a firmware update.

While the API itself is perfectly able to handle NATIVE mode the keyid 
is not handled correctly. Installing a second key switches TX to the new 
key and overwrites the keyid in the MPDU mac80211 prepared. (I could not 
even get the card to properly make an RX/TX key to an TX only key, that 
caused clear text packets when changing the key and it looks like that 
SW crypto is only possible - with nonfree CT - when not using HW crypto 
for TX at all. With those limitations I shelved any plans for ath10k.)

One of my next planned steps is now to either get another ath9k card or 
get another driver working in AP mode to test ath9k also in managed 
mode. Of course I also have to get sniffing working properly, all cards 
tried so far have issues and it also looks like I have to update 
wireshark for serious testing. So I guess driver support will still take 
some time and efforts when we got the generic issues sorted out.

I can also try my hand at porting the other Atheros drives, but without 
someone being able to confirm it works I'm not planning that at the moment.

Alexander






^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC PATCH v3 02/12] nl80211/cfg80211: Extended Key ID support
  2019-02-10 21:06 ` [RFC PATCH v3 02/12] nl80211/cfg80211: Extended Key ID support Alexander Wetzel
@ 2019-02-15 10:52   ` Johannes Berg
  2019-02-17 19:19     ` Alexander Wetzel
  0 siblings, 1 reply; 23+ messages in thread
From: Johannes Berg @ 2019-02-15 10:52 UTC (permalink / raw)
  To: Alexander Wetzel; +Cc: linux-wireless

On Sun, 2019-02-10 at 22:06 +0100, Alexander Wetzel wrote:
> +/**
> + * enum nl80211_key_install_mode - Key install mode
> + *
> + * @NL80211_KEY_RX_TX: Key must be installed for Rx and Tx
> + * @NL80211_KEY_RX_ONLY: Allowed in combination with @NL80211_CMD_NEW_KEY:
> + *	Unicast key has to be installed for Rx only.
> + * @NL80211_KEY_SWITCH_TX: Allowed in combination with @NL80211_CMD_SET_KEY:
> + *	Switch Tx to a Rx only, referenced by sta mac and idx.

Don't you mean the other way around? Or, well, what you say is true for
the *other* keys?

>   *	by the %NL80211_SCAN_FLAG_MIN_PREQ_CONTENT flag.
>   * @NL80211_EXT_FEATURE_ENABLE_FTM_RESPONDER: Driver supports enabling fine
>   *	timing measurement responder role.
> - *

no need to remove that :)

> -	/* only support setting default key */
> -	if (!key.def && !key.defmgmt)
> +	/* Only support setting default key and
> +	 * Extended Key ID action @NL80211_KEY_SWITCH_TX.
> +	 */

you can remove the @, it's not a kernel-doc formatted comment

> -	}
> +	} else if (key.p.install_mode == NL80211_KEY_SWITCH_TX &&
> +		   wiphy_ext_feature_isset(&rdev->wiphy,
> +					   NL80211_EXT_FEATURE_EXT_KEY_ID)) {
> +		u8 *mac_addr = NULL;
>  
> +		if (info->attrs[NL80211_ATTR_MAC])
> +			mac_addr = nla_data(info->attrs[NL80211_ATTR_MAC]);
> +
> +		if (!mac_addr || key.idx < 0 || key.idx > 1) {
> +			err = -EINVAL;
> +			goto out;
> +		}

Really only 0 and 1 are allowed? Not 0-3?

> +++ b/net/wireless/util.c
> @@ -236,14 +236,22 @@ int cfg80211_validate_key_settings(struct cfg80211_registered_device *rdev,
>  	case WLAN_CIPHER_SUITE_CCMP_256:
>  	case WLAN_CIPHER_SUITE_GCMP:
>  	case WLAN_CIPHER_SUITE_GCMP_256:
> -		/* Disallow pairwise keys with non-zero index unless it's WEP
> -		 * or a vendor specific cipher (because current deployments use
> -		 * pairwise WEP keys with non-zero indices and for vendor
> -		 * specific ciphers this should be validated in the driver or
> -		 * hardware level - but 802.11i clearly specifies to use zero)
> +		/* IEEE802.11-2016 allows only 0 and - when using Extended Key
> +		 * ID - 1 as index for pairwise keys.
> +		 * @NL80211_KEY_RX_ONLY is only allowed for pairwise keys when
> +		 * the driver supports Extended Key ID.
> +		 * @NL80211_KEY_SWITCH_TX must not be set when validating a key.
>  		 */
> -		if (pairwise && key_idx)
> +		if (params->install_mode == NL80211_KEY_RX_ONLY) {
> +			if (!wiphy_ext_feature_isset(&rdev->wiphy,
> +						     NL80211_EXT_FEATURE_EXT_KEY_ID))
> +				return -EINVAL;
> +			else if (!pairwise || key_idx < 0 || key_idx > 1)
> +				return -EINVAL;

same question here

johannes


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC PATCH v3 03/12] mac80211: IEEE 802.11 Extended Key ID support
  2019-02-10 21:06 ` [RFC PATCH v3 03/12] mac80211: IEEE 802.11 " Alexander Wetzel
@ 2019-02-15 11:06   ` Johannes Berg
  0 siblings, 0 replies; 23+ messages in thread
From: Johannes Berg @ 2019-02-15 11:06 UTC (permalink / raw)
  To: Alexander Wetzel; +Cc: linux-wireless

On Sun, 2019-02-10 at 22:06 +0100, Alexander Wetzel wrote:
> 
>  - Enforce cipher does not change when replacing a key.

is that actually required somehow?

> + * @EXT_SET_KEY: a new key must be set but is only valid for decryption
> + * @EXT_KEY_RX_TX: a key installed with @EXT_SET_KEY is becoming the
> + *	designated Rx/Tx key for the station

Not sure I like the EXT_SET_KEY. There's also no "designated Rx key", is
there? It's always selected by key ID.

How about SET_KEY_RXONLY and SET_KEY_TX or something like that?

> +static int ieee80211_set_tx_key(struct ieee80211_sub_if_data *sdata,
> +				const u8 *mac_addr, u8 key_idx)
> +{
> +	struct ieee80211_local *local = sdata->local;
> +	struct ieee80211_key *key;
> +	struct sta_info *sta;
> +	int ret;
> +
> +	if (!wiphy_ext_feature_isset(local->hw.wiphy,
> +				     NL80211_EXT_FEATURE_EXT_KEY_ID))
> +		return -EINVAL;

You set this, wouldn't it make more sense to check EXT_KEY_ID_NATIVE?

Or maybe this is because of the next patch?

> +	sta = sta_info_get_bss(sdata, mac_addr);
> +
> +	if (!sta)
> +		return -EINVAL;
> +
> +	if (sta->ptk_idx == key_idx)
> +		return 0;
> +
> +	mutex_lock(&local->key_mtx);
> +	key = key_mtx_dereference(local, sta->ptk[key_idx]);
> +
> +	if (key && key->flags & KEY_FLAG_RX_ONLY)

do you even need the flag? Isn't it equivalent to checking
	sta->ptk_idx != key->idx
or so?

Less data to maintain would be better.

> +	bool ext_native = ieee80211_hw_check(&local->hw, EXT_KEY_ID_NATIVE);

you sort of only need this in the next patch, but I guess it doesn't
matter that much

> +int ieee80211_key_activate_tx(struct ieee80211_key *key)
> +{
> +	struct ieee80211_sub_if_data *sdata = key->sdata;
> +	struct sta_info *sta = key->sta;
> +	struct ieee80211_local *local = key->local;
> +	struct ieee80211_key *old;
> +	int ret;
> +
> +	assert_key_lock(local);
> +
> +	key->flags &= ~KEY_FLAG_RX_ONLY;
> +
> +	if (!(key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) ||
> +	    key->conf.flags & (IEEE80211_KEY_FLAG_GENERATE_MMIC |
> +			       IEEE80211_KEY_FLAG_PUT_MIC_SPACE |
> +			       IEEE80211_KEY_FLAG_RESERVE_TAILROOM))
> +		increment_tailroom_need_count(sdata);
> +
> +	if (key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) {
> +		ret = drv_set_key(local, EXT_KEY_RX_TX, sdata,
> +				  &sta->sta, &key->conf);
> +		if (ret) {
> +			sdata_err(sdata,
> +				  "failed to activate key for Tx (%d, %pM)\n",
> +				  key->conf.keyidx, sta->sta.addr);
> +			return ret;

You've already cleared the RX_ONLY flag, which gets you inconsistent
data now.

> +		}
> +	}
> +
> +	old = key_mtx_dereference(local, sta->ptk[sta->ptk_idx]);
> +	sta->ptk_idx = key->conf.keyidx;

but you set this only here.

> -		/* Stop TX till we are on the new key */
> +		/* Stop Tx till we are on the new key */

Uh, I had to read that three times ... please don't make changes like
that? :)

>  		old_key->flags |= KEY_FLAG_TAINTED;
>  		ieee80211_clear_fast_xmit(sta);
>  
> -		/* Aggregation sessions during rekey are complicated due to the
> +		/* Aggregation sessions during rekey are complicated by the

similarly here, please don't make drive-by comment wording issues (also,
I'm not sure I agree - the old version just treats "complicated" as an
adjective, you treat it as a verb, but ultimately doesn't really matter?

>  #define NUM_DEFAULT_KEYS 4
>  #define NUM_DEFAULT_MGMT_KEYS 2
> +#define INVALID_PTK_KEYIDX 2 /* Existing key slot never used by PTK keys */

We could also use something obviously wrong like 0xff?

> +++ b/net/mac80211/tx.c
> @@ -3000,23 +3000,15 @@ void ieee80211_check_fast_xmit(struct sta_info *sta)
>  		switch (build.key->conf.cipher) {
>  		case WLAN_CIPHER_SUITE_CCMP:
>  		case WLAN_CIPHER_SUITE_CCMP_256:
> -			/* add fixed key ID */
> -			if (gen_iv) {
> -				(build.hdr + build.hdr_len)[3] =
> -					0x20 | (build.key->conf.keyidx << 6);
> +			if (gen_iv)
>  				build.pn_offs = build.hdr_len;
> -			}
>  			if (gen_iv || iv_spc)
>  				build.hdr_len += IEEE80211_CCMP_HDR_LEN;
>  			break;
>  		case WLAN_CIPHER_SUITE_GCMP:
>  		case WLAN_CIPHER_SUITE_GCMP_256:
> -			/* add fixed key ID */
> -			if (gen_iv) {
> -				(build.hdr + build.hdr_len)[3] =
> -					0x20 | (build.key->conf.keyidx << 6);
> +			if (gen_iv)
>  				build.pn_offs = build.hdr_len;
> -			}
>  			if (gen_iv || iv_spc)
>  				build.hdr_len += IEEE80211_GCMP_HDR_LEN;
>  			break;
> @@ -3383,6 +3375,7 @@ static void ieee80211_xmit_fast_finish(struct ieee80211_sub_if_data *sdata,
>  			pn = atomic64_inc_return(&key->conf.tx_pn);
>  			crypto_hdr[0] = pn;
>  			crypto_hdr[1] = pn >> 8;
> +			crypto_hdr[3] = 0x20 | (key->conf.keyidx << 6);
>  			crypto_hdr[4] = pn >> 16;
>  			crypto_hdr[5] = pn >> 24;
>  			crypto_hdr[6] = pn >> 32;

This shouldn't be needed, you do update the fast TX cache when changing
the key?

johannes


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC PATCH v3 04/12] mac80211: Compatibility Extended Key ID support
  2019-02-10 21:06 ` [RFC PATCH v3 04/12] mac80211: Compatibility " Alexander Wetzel
@ 2019-02-15 11:09   ` Johannes Berg
  0 siblings, 0 replies; 23+ messages in thread
From: Johannes Berg @ 2019-02-15 11:09 UTC (permalink / raw)
  To: Alexander Wetzel; +Cc: linux-wireless


> +	if (!ext_native && key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) {
> +		key->flags |= KEY_FLAG_RX_SW_CRYPTO;
> +		/* Activate Rx crypto offload after max 10s when idle */
> +		ieee80211_queue_delayed_work(&local->hw, &sta->ext_key_compat_wk,
> +					     round_jiffies_relative(HZ * 10));
> +	}

Is there much point in this?

> +		if (unlikely(rx->key->flags & KEY_FLAG_RX_SW_CRYPTO)) {
> +			rx->key->flags &= ~KEY_FLAG_RX_SW_CRYPTO;
> +			cancel_delayed_work(&rx->sta->ext_key_compat_wk);
> +			ieee80211_queue_delayed_work(&rx->local->hw,
> +						     &rx->sta->ext_key_compat_wk, 0);
> +		}

We'll almost certainly do it from here, so never exercise the other
path?

johannes


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC PATCH v3 00/12] Draft for Extended Key ID support
  2019-02-10 21:06 [RFC PATCH v3 00/12] Draft for Extended Key ID support Alexander Wetzel
                   ` (11 preceding siblings ...)
  2019-02-10 21:06 ` [RFC PATCH v3 12/12] ath9k: EXT_KEY_ID A-MPDU API update Alexander Wetzel
@ 2019-02-15 11:10 ` Johannes Berg
  12 siblings, 0 replies; 23+ messages in thread
From: Johannes Berg @ 2019-02-15 11:10 UTC (permalink / raw)
  To: Alexander Wetzel; +Cc: linux-wireless

On Sun, 2019-02-10 at 22:06 +0100, Alexander Wetzel wrote:
> 
> The driver patches are - with the exception of the hwsim patch -
> definitely not ready for merge and mostly here to illustrate how the
> different APIs can be used and to start some discussions how to handle HW
> specific challenges. Of course if someone wants to play with Extended Key
> ID they also should be useful... (I can provide updated mostly working
> hostapd/wpa_supplicant patches if someone is interested.

Of course :-)

Some tests for the hwsim tests there would also be nice, that's the
easiest way to see something working - if you have them.

johannes


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC PATCH v3 05/12] mac80211: Mark A-MPDU keyid borders for drivers
  2019-02-10 21:06 ` [RFC PATCH v3 05/12] mac80211: Mark A-MPDU keyid borders for drivers Alexander Wetzel
@ 2019-02-15 11:50   ` Johannes Berg
  0 siblings, 0 replies; 23+ messages in thread
From: Johannes Berg @ 2019-02-15 11:50 UTC (permalink / raw)
  To: Alexander Wetzel; +Cc: linux-wireless


> The intent of the wording was probably written without considering
> Extended Key IDs. At least it makes no sense for me to forbid mixing
> MPDUs using keyid 0 and 1 in one A-MPDU. 

I think it may make sense - reprogramming the hardware engines may take
some time, and doing that in the middle of the A-MPDU may not be
feasible? You don't just have to load the key (that you need to do
anyway) but also extract the status? I dunno, I'm more handwaving, but
it doesn't make sense to add such a requirement when only one key index
can be used to start with.

> The code is assuming that the driver is not aggregating MPDUs more than
> 5s apart. We probably don't have wait nearly so long but I'm not sure
> what is the minimum time.

OTOH, if you have a lot of BE/VI/VO traffic BK might be starved even
longer than that, technically indefinitely.

> +static struct ieee80211_key debug_noinline
> +*ieee80211_select_sta_key(struct ieee80211_tx_data *tx)
> +{
> +	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)tx->skb->data;
> +	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx->skb);
> +	struct sta_info *sta = tx->sta;
> +	struct ieee80211_key *key;
> +	struct ieee80211_key *next_key;
> +
> +	key = rcu_dereference(tx->sta->ptk[tx->sta->ptk_idx]);
> +
> +	if (likely(sta->ptk_idx_next == INVALID_PTK_KEYIDX))
> +		return key;
> +
> +	/* Only when using Extended Key ID the code below can be executed */
> +
> +	if (!ieee80211_is_data_present(hdr->frame_control))
> +		return key;
> +
> +	if (sta->ptk_idx_next == sta->ptk_idx) {
> +		/* First packet using new key with A-MPDU active*/
> +		sta->ptk_idx_next = INVALID_PTK_KEYIDX;
> +		ieee80211_check_fast_xmit(tx->sta);

I'm not convinced you can call this from this context? It looks safe
though, but it's really strange in a way.

> +	info->flags &= ~IEEE80211_TX_CTL_AMPDU;

Like you say above, I don't think this really makes a lot of sense. If
we don't have any free bits I guess we should try to find some ...

johannes


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC PATCH v3 07/12] iwlwifi: Extended Key ID support (NATIVE)
  2019-02-10 21:06 ` [RFC PATCH v3 07/12] iwlwifi: Extended " Alexander Wetzel
@ 2019-02-15 11:52   ` Johannes Berg
  0 siblings, 0 replies; 23+ messages in thread
From: Johannes Berg @ 2019-02-15 11:52 UTC (permalink / raw)
  To: Alexander Wetzel; +Cc: linux-wireless

On Sun, 2019-02-10 at 22:06 +0100, Alexander Wetzel wrote:
> This is not ready for merge and has known issues.
> The patch is only for discussions to sort out how to handle it correctly!
> 
> Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de>
> ---
> 
> iwlwifi intel cards had two big surprises:
> 
> Assuming I did not make some stupid errors it looks like my old
> "Intel Corporation Centrino Ultimate-N 6300 (rev 3e)" using ucode 
> 9.221.4.1 build 25532 is perfectly fine with two keys uploaded to
> harware and honoring the keyid in the MPDUs. For a card launched 2011
> that's a pleasant surprise:-)

:-)

> A much shorter test with a modern "Intel Corporation Wireless 8265 /
> 8175 (rev 78)" using ucode version 36.e91976c0.0 shows what seems to be
> MPDUs decoded with the wrong key at each rekey and therefore a candidate
> for the COMPAT support only..
> So the bad news seems to be, that the modern card dropped the support.

Probably just a firmware bug.

> It also seems to force us to add some per-card or per-firmware depending
> check to decide which card can use the Native Extended Key ID support
> and use the Compat mode for the rest.
> Is there any way to find out which cards/firmware can be used with
> Extended Key ID?

No, but if you have a good test case we can check out what the firmware
bug is and fix it. Perhaps not for all, but for the future at least.
Maybe we can still figure out where it was introduced and thus see where
it's good to use native mode.

> I also have tested patch for iwldvm using the Compat mode and I think
> mvm cards will also work with that.

No they don't, no firmware is available for that.

johannes


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC PATCH v3 08/12] iwlwifi: dvm - EXT_KEY_ID A-MPDU API update
  2019-02-10 21:06 ` [RFC PATCH v3 08/12] iwlwifi: dvm - EXT_KEY_ID A-MPDU API update Alexander Wetzel
@ 2019-02-15 11:54   ` Johannes Berg
  0 siblings, 0 replies; 23+ messages in thread
From: Johannes Berg @ 2019-02-15 11:54 UTC (permalink / raw)
  To: Alexander Wetzel; +Cc: linux-wireless

On Sun, 2019-02-10 at 22:06 +0100, Alexander Wetzel wrote:
> When using Extended Key ID mac80211 drops @IEEE80211_TX_CTL_AMPDU for
> the last packet which can be added to a A-MPDU in preparation.
> 
> Don't throw a warning and just handle the frame as if
> @IEEE80211_TX_CTL_AMPDU would have been set.
> 
> Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de>
> ---
> 
> I cold not figure out so far how to make sure the card will not mix
> A-MPDU frames. Looks like that is handled fully in the HW and I didn't
> find any interface to influence it. (It even may work already, I have
> some problems to capture A-MPDU frames with A-MPDU information intact
> and the topic was pretty low on the list so far. After all it works...)

You can't really do that, as far as I can tell, unfortunately. So this
might be better in a "compat on steroids" mode?

johannes



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC PATCH v3 02/12] nl80211/cfg80211: Extended Key ID support
  2019-02-15 10:52   ` Johannes Berg
@ 2019-02-17 19:19     ` Alexander Wetzel
  0 siblings, 0 replies; 23+ messages in thread
From: Alexander Wetzel @ 2019-02-17 19:19 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless

>> +/**
>> + * enum nl80211_key_install_mode - Key install mode
>> + *
>> + * @NL80211_KEY_RX_TX: Key must be installed for Rx and Tx
>> + * @NL80211_KEY_RX_ONLY: Allowed in combination with @NL80211_CMD_NEW_KEY:
>> + *	Unicast key has to be installed for Rx only.
>> + * @NL80211_KEY_SWITCH_TX: Allowed in combination with @NL80211_CMD_SET_KEY:
>> + *	Switch Tx to a Rx only, referenced by sta mac and idx.
> 
> Don't you mean the other way around? Or, well, what you say is true for
> the *other* keys?

I don't see the problem but I may simply be to set on my way to see 
it... So I'll just give an outline what's required of the API and how 
this implementation meets those. Hoping that answers your question or 
allowing you to pinpoint our differences in understanding. (I've added a 
  more than really required, it may be useful for other discussions to 
have that outlined in one piece, too.)

Extended Key ID has only two requirements for the kernel API:
1) Allow a key to be used for decryption first and switch it to a 
"normal" Rx/Tx key with a second call

2) Allow pairwise keys to use keyids 0 and 1 instead only 0.

"Legacy" key installs are using NL80211_CMD_NEW_KEY to install a new key 
and are allow to mark a key for WEP or management  default usages via 
NL80211_CMD_SET_KEY later.

With Extended Key ID we stick to a similar approach: NL80211_CMD_NEW_KEY 
is called to install the new key and nl80211_key_install_mode allows to 
select between a "legacy" or "extended" key install: NL80211_KEY_RX_TX 
for "legacy" or NL80211_KEY_RX_ONLY for "extended".

NL80211_KEY_SWITCH_TX is only allowed with NL80211_CMD_SET_KEY and is 
the only Extended Key ID action allowed for that function.

Any non-pairwise keys can only use NL80211_KEY_RX_TX which is of course 
always the default and also allowed for "legacy" pairwise keys.

A Extended Key Install will:
1) call NL80211_CMD_NEW_KEY with all the usual parameters of a new key 
install plus the flag NL80211_KEY_RX_ONLY set.

2) Once ready will call NL80211_CMD_SET_KEY with flag 
NL80211_KEY_SWITCH_TX to activate a specific key.


>>    *	by the %NL80211_SCAN_FLAG_MIN_PREQ_CONTENT flag.
>>    * @NL80211_EXT_FEATURE_ENABLE_FTM_RESPONDER: Driver supports enabling fine
>>    *	timing measurement responder role.
>> - *
> 
> no need to remove that :)

ok, I'll remove all the drive-by comment "cleanups".
It (often) looks kind of untidy and not really worth separate patches 
and I'm kind of responsible for (most) of them.. I see why you don't 
want to see those fixed drive-by...

My understanding is now that we prefer to not change those and I'll 
leave them alone.

> 
>> -	/* only support setting default key */
>> -	if (!key.def && !key.defmgmt)
>> +	/* Only support setting default key and
>> +	 * Extended Key ID action @NL80211_KEY_SWITCH_TX.
>> +	 */
> 
> you can remove the @, it's not a kernel-doc formatted comment
> 
>> -	}
>> +	} else if (key.p.install_mode == NL80211_KEY_SWITCH_TX &&
>> +		   wiphy_ext_feature_isset(&rdev->wiphy,
>> +					   NL80211_EXT_FEATURE_EXT_KEY_ID)) {
>> +		u8 *mac_addr = NULL;
>>   
>> +		if (info->attrs[NL80211_ATTR_MAC])
>> +			mac_addr = nla_data(info->attrs[NL80211_ATTR_MAC]);
>> +
>> +		if (!mac_addr || key.idx < 0 || key.idx > 1) {
>> +			err = -EINVAL;
>> +			goto out;
>> +		}
> 
> Really only 0 and 1 are allowed? Not 0-3?

Yes. Extended Key ID only allows to use Key ID 1 on top of the 
established ID 0. See "IEEE 802.11 - 2016 9.4.2.25.4 RSN capabilities":

Bit 13: Extended Key ID for Individually Addressed Frames. This subfield 
is set to 1 to indicate that the STA supports Key ID values in the range 
0 to 1 for a PTKSA and STKSA when the cipher suite is CCMP or GCMP. A 
value of 0 indicates that the STA only supports Key ID 0 for a PTKSA and
STKSA.

Or also "12.7.6.4 4-way handshake message 2":

If the Extended Key ID for Individually Addressed Frames subfield of the 
RSN Capabilities field is 1 for both the Authenticator and the 
Supplicant, then the Authenticator assigns a new Key ID for the PTKSA in
the range 0 to 1 that is different from the Key ID assigned in the 
previous handshake and uses the MLME-SETKEYS.request primitive to 
install the new key to receive individually addressed MPDUs protected by
the PTK with the assigned Key ID.

> 
>> +++ b/net/wireless/util.c
>> @@ -236,14 +236,22 @@ int cfg80211_validate_key_settings(struct cfg80211_registered_device *rdev,
>>   	case WLAN_CIPHER_SUITE_CCMP_256:
>>   	case WLAN_CIPHER_SUITE_GCMP:
>>   	case WLAN_CIPHER_SUITE_GCMP_256:
>> -		/* Disallow pairwise keys with non-zero index unless it's WEP
>> -		 * or a vendor specific cipher (because current deployments use
>> -		 * pairwise WEP keys with non-zero indices and for vendor
>> -		 * specific ciphers this should be validated in the driver or
>> -		 * hardware level - but 802.11i clearly specifies to use zero)
>> +		/* IEEE802.11-2016 allows only 0 and - when using Extended Key
>> +		 * ID - 1 as index for pairwise keys.
>> +		 * @NL80211_KEY_RX_ONLY is only allowed for pairwise keys when
>> +		 * the driver supports Extended Key ID.
>> +		 * @NL80211_KEY_SWITCH_TX must not be set when validating a key.
>>   		 */
>> -		if (pairwise && key_idx)
>> +		if (params->install_mode == NL80211_KEY_RX_ONLY) {
>> +			if (!wiphy_ext_feature_isset(&rdev->wiphy,
>> +						     NL80211_EXT_FEATURE_EXT_KEY_ID))
>> +				return -EINVAL;
>> +			else if (!pairwise || key_idx < 0 || key_idx > 1)
>> +				return -EINVAL;
> 
> same question here

same answer with one remark: The original code did only allow id 0 and 
and I made sure only with install mode NL80211_KEY_RX_ONLY is allowed to 
use non-zero IDs.

Alexander

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, back to index

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-10 21:06 [RFC PATCH v3 00/12] Draft for Extended Key ID support Alexander Wetzel
2019-02-10 21:06 ` [RFC PATCH v3 01/12] mac80211: Optimize tailroom_needed update checks Alexander Wetzel
2019-02-10 21:06 ` [RFC PATCH v3 02/12] nl80211/cfg80211: Extended Key ID support Alexander Wetzel
2019-02-15 10:52   ` Johannes Berg
2019-02-17 19:19     ` Alexander Wetzel
2019-02-10 21:06 ` [RFC PATCH v3 03/12] mac80211: IEEE 802.11 " Alexander Wetzel
2019-02-15 11:06   ` Johannes Berg
2019-02-10 21:06 ` [RFC PATCH v3 04/12] mac80211: Compatibility " Alexander Wetzel
2019-02-15 11:09   ` Johannes Berg
2019-02-10 21:06 ` [RFC PATCH v3 05/12] mac80211: Mark A-MPDU keyid borders for drivers Alexander Wetzel
2019-02-15 11:50   ` Johannes Berg
2019-02-10 21:06 ` [RFC PATCH v3 06/12] mac80211_hwsim: Ext Key ID support (NATIVE) Alexander Wetzel
2019-02-10 21:06 ` [RFC PATCH v3 07/12] iwlwifi: Extended " Alexander Wetzel
2019-02-15 11:52   ` Johannes Berg
2019-02-10 21:06 ` [RFC PATCH v3 08/12] iwlwifi: dvm - EXT_KEY_ID A-MPDU API update Alexander Wetzel
2019-02-15 11:54   ` Johannes Berg
2019-02-10 21:06 ` [RFC PATCH v3 09/12] ath: Basic Extended Key ID support (COMPAT+NATIVE) Alexander Wetzel
2019-02-13 11:05   ` Kalle Valo
2019-02-13 23:15     ` Alexander Wetzel
2019-02-10 21:06 ` [RFC PATCH v3 10/12] ath5k: ath_key_config() API compatibility update Alexander Wetzel
2019-02-10 21:06 ` [RFC PATCH v3 11/12] ath9k: Extended Key ID support (COMPAT) Alexander Wetzel
2019-02-10 21:06 ` [RFC PATCH v3 12/12] ath9k: EXT_KEY_ID A-MPDU API update Alexander Wetzel
2019-02-15 11:10 ` [RFC PATCH v3 00/12] Draft for Extended Key ID support Johannes Berg

Linux-Wireless Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-wireless/0 linux-wireless/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-wireless linux-wireless/ https://lore.kernel.org/linux-wireless \
		linux-wireless@vger.kernel.org linux-wireless@archiver.kernel.org
	public-inbox-index linux-wireless


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-wireless


AGPL code for this site: git clone https://public-inbox.org/ public-inbox