diff mbox series

[v8,04/15] drivers: Migrate drivers from set_tx to key_type API

Message ID 20191031091901.2889-5-alexander@wetzel-home.de
State Changes Requested
Headers show
Series Support seamless PTK rekeys with Extended Key ID | expand

Commit Message

Alexander Wetzel Oct. 31, 2019, 9:18 a.m. UTC
Migrate all drivers except nl80211 from set_tx to key_type.

This patch assumes that the drivers only are using set_tx for default
keys as it was (probably) intended and are not relaying on the quirks
set_tx really has.

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

This is in my opinion the most risky part of retiring the set_tx API.
I have zero knowledge about those drivers and the new key_type API is
quite often using KEY_TYPE_BROADCAST where set_tx was set to 1. While
the changes are logical I would not be surprised if this patch would
cause one or another issue when using WEP or the default key.

Now I really would like to fully clean up set_tx and fully switch to the
new key_flag to not clutter the API. The way the patches are structured
we could keep set_tx around and only migrate nl80211 to the new API...
But this would be quite confusing and sooner or later someone will mix
up the two APIs again, making it even worse to fix next time.

As a side note: I first "extended" the existing "boolean" set_tx to a
bit field. But the sometimes incomprehensible usage of set_tx and the
investigation how it should be done expanded the scope to what you see
here.
For the older approach see
https://patchwork.ozlabs.org/project/hostap/list/?series=75270.

 src/drivers/driver_atheros.c |  6 +++---
 src/drivers/driver_bsd.c     |  8 ++++----
 src/drivers/driver_hostap.c  |  3 ++-
 src/drivers/driver_ndis.c    | 10 ++++++----
 src/drivers/driver_nl80211.c |  5 ++++-
 src/drivers/driver_privsep.c |  6 +++---
 src/drivers/driver_wext.c    | 11 +++++++----
 7 files changed, 29 insertions(+), 20 deletions(-)
diff mbox series

Patch

diff --git a/src/drivers/driver_atheros.c b/src/drivers/driver_atheros.c
index e95652de5..cd35ea9b3 100644
--- a/src/drivers/driver_atheros.c
+++ b/src/drivers/driver_atheros.c
@@ -561,7 +561,7 @@  atheros_set_key(const char *ifname, void *priv, enum wpa_alg alg,
 	if (addr == NULL || is_broadcast_ether_addr(addr)) {
 		os_memset(wk.ik_macaddr, 0xff, IEEE80211_ADDR_LEN);
 		wk.ik_keyix = key_idx;
-		if (set_tx)
+		if (key_type == KEY_TYPE_DEFAULT)
 			wk.ik_flags |= IEEE80211_KEY_DEFAULT;
 	} else {
 		os_memcpy(wk.ik_macaddr, addr, IEEE80211_ADDR_LEN);
@@ -573,9 +573,9 @@  atheros_set_key(const char *ifname, void *priv, enum wpa_alg alg,
 	ret = set80211priv(drv, IEEE80211_IOCTL_SETKEY, &wk, sizeof(wk));
 	if (ret < 0) {
 		wpa_printf(MSG_DEBUG, "%s: Failed to set key (addr %s"
-			   " key_idx %d alg %d key_len %lu set_tx %d)",
+			   " key_idx %d alg %d key_len %lu key_type %d)",
 			   __func__, ether_sprintf(wk.ik_macaddr), key_idx,
-			   alg, (unsigned long) key_len, set_tx);
+			   alg, (unsigned long) key_len, key_type);
 	}
 
 	return ret;
diff --git a/src/drivers/driver_bsd.c b/src/drivers/driver_bsd.c
index de9fa6b02..07d9bcf69 100644
--- a/src/drivers/driver_bsd.c
+++ b/src/drivers/driver_bsd.c
@@ -341,9 +341,9 @@  bsd_set_key(const char *ifname, void *priv, enum wpa_alg alg,
 	struct bsd_driver_data *drv = priv;
 #endif /* IEEE80211_KEY_NOREPLAY */
 
-	wpa_printf(MSG_DEBUG, "%s: alg=%d addr=%p key_idx=%d set_tx=%d "
+	wpa_printf(MSG_DEBUG, "%s: alg=%d addr=%p key_idx=%d key_type=%d "
 		   "seq_len=%zu key_len=%zu", __func__, alg, addr, key_idx,
-		   set_tx, seq_len, key_len);
+		   key_type, seq_len, key_len);
 
 	if (alg == WPA_ALG_NONE) {
 #ifndef HOSTAPD
@@ -371,7 +371,7 @@  bsd_set_key(const char *ifname, void *priv, enum wpa_alg alg,
 	}
 
 	wk.ik_flags = IEEE80211_KEY_RECV;
-	if (set_tx)
+	if (key_type == KEY_TYPE_DEFAULT)
 		wk.ik_flags |= IEEE80211_KEY_XMIT;
 
 	if (addr == NULL) {
@@ -392,7 +392,7 @@  bsd_set_key(const char *ifname, void *priv, enum wpa_alg alg,
 				key_idx;
 		}
 	}
-	if (wk.ik_keyix != IEEE80211_KEYIX_NONE && set_tx)
+	if (wk.ik_keyix != IEEE80211_KEYIX_NONE && key_type == KEY_TYPE_DEFAULT)
 		wk.ik_flags |= IEEE80211_KEY_DEFAULT;
 #ifndef HOSTAPD
 #ifdef IEEE80211_KEY_NOREPLAY
diff --git a/src/drivers/driver_hostap.c b/src/drivers/driver_hostap.c
index bf22858fb..454388fe7 100644
--- a/src/drivers/driver_hostap.c
+++ b/src/drivers/driver_hostap.c
@@ -440,7 +440,8 @@  static int wpa_driver_hostap_set_key(const char *ifname, void *priv,
 		os_free(buf);
 		return -1;
 	}
-	param->u.crypt.flags = set_tx ? HOSTAP_CRYPT_FLAG_SET_TX_KEY : 0;
+	param->u.crypt.flags = key_type == KEY_TYPE_DEFAULT ?
+			       HOSTAP_CRYPT_FLAG_SET_TX_KEY : 0;
 	param->u.crypt.idx = key_idx;
 	param->u.crypt.key_len = key_len;
 	memcpy((u8 *) (param + 1), key, key_len);
diff --git a/src/drivers/driver_ndis.c b/src/drivers/driver_ndis.c
index 649bc01ea..2963e1f51 100644
--- a/src/drivers/driver_ndis.c
+++ b/src/drivers/driver_ndis.c
@@ -945,7 +945,7 @@  static int wpa_driver_ndis_add_wep(struct wpa_driver_ndis_data *drv,
 		return -1;
 	wep->Length = len;
 	wep->KeyIndex = key_idx;
-	if (set_tx)
+	if (key_type == KEY_TYPE_DEFAULT)
 		wep->KeyIndex |= 1 << 31;
 #if 0 /* Setting bit30 does not seem to work with some NDIS drivers */
 	if (pairwise)
@@ -1006,7 +1006,7 @@  static int wpa_driver_ndis_set_key(const char *ifname, void *priv,
 
 	nkey->Length = len;
 	nkey->KeyIndex = key_idx;
-	if (set_tx)
+	if (key_type == KEY_TYPE_DEFAULT)
 		nkey->KeyIndex |= 1 << 31;
 	if (pairwise)
 		nkey->KeyIndex |= 1 << 30;
@@ -1077,7 +1077,8 @@  wpa_driver_ndis_associate(void *priv,
 						bcast, i,
 						i == params->wep_tx_keyidx,
 						NULL, 0, params->wep_key[i],
-						params->wep_key_len[i], 0);
+						params->wep_key_len[i],
+						KEY_TYPE_BROADCAST);
 		}
 	}
 
@@ -1114,7 +1115,8 @@  wpa_driver_ndis_associate(void *priv,
 			wpa_driver_ndis_set_key(drv->ifname, drv, WPA_ALG_WEP,
 						bcast, 0, 1,
 						NULL, 0, dummy_key,
-						sizeof(dummy_key), 0);
+						sizeof(dummy_key),
+						KEY_TYPE_BROADCAST);
 		}
 #endif /* CONFIG_WPS */
 	} else {
diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
index 830e37a54..e4829c84d 100644
--- a/src/drivers/driver_nl80211.c
+++ b/src/drivers/driver_nl80211.c
@@ -3486,7 +3486,10 @@  retry:
 					   NULL, i,
 					   i == params->wep_tx_keyidx, NULL, 0,
 					   params->wep_key[i],
-					   params->wep_key_len[i], 0);
+					   params->wep_key_len[i],
+					   i == params->wep_tx_keyidx ?
+					   KEY_TYPE_DEFAULT :
+					   KEY_TYPE_BROADCAST);
 		if (params->wep_tx_keyidx != i)
 			continue;
 		if (nl_add_key(msg, WPA_ALG_WEP, i, 1, NULL, 0,
diff --git a/src/drivers/driver_privsep.c b/src/drivers/driver_privsep.c
index e3375cd90..b3d2ddae0 100644
--- a/src/drivers/driver_privsep.c
+++ b/src/drivers/driver_privsep.c
@@ -215,8 +215,8 @@  static int wpa_driver_privsep_set_key(const char *ifname, void *priv,
 	struct wpa_driver_privsep_data *drv = priv;
 	struct privsep_cmd_set_key cmd;
 
-	wpa_printf(MSG_DEBUG, "%s: priv=%p alg=%d key_idx=%d set_tx=%d",
-		   __func__, priv, alg, key_idx, set_tx);
+	wpa_printf(MSG_DEBUG, "%s: priv=%p alg=%d key_idx=%d key_type=%d",
+		   __func__, priv, alg, key_idx, key_type);
 
 	os_memset(&cmd, 0, sizeof(cmd));
 	cmd.alg = alg;
@@ -225,7 +225,7 @@  static int wpa_driver_privsep_set_key(const char *ifname, void *priv,
 	else
 		os_memset(cmd.addr, 0xff, ETH_ALEN);
 	cmd.key_idx = key_idx;
-	cmd.set_tx = set_tx;
+	cmd.set_tx = key_type == KEY_TYPE_DEFAULT;
 	if (seq && seq_len > 0 && seq_len < sizeof(cmd.seq)) {
 		os_memcpy(cmd.seq, seq, seq_len);
 		cmd.seq_len = seq_len;
diff --git a/src/drivers/driver_wext.c b/src/drivers/driver_wext.c
index 5008bdc99..aff919ba3 100644
--- a/src/drivers/driver_wext.c
+++ b/src/drivers/driver_wext.c
@@ -1740,7 +1740,7 @@  static int wpa_driver_wext_set_key_ext(void *priv, enum wpa_alg alg,
 
 	if (addr == NULL || is_broadcast_ether_addr(addr))
 		ext->ext_flags |= IW_ENCODE_EXT_GROUP_KEY;
-	if (set_tx)
+	if (key_type == KEY_TYPE_DEFAULT)
 		ext->ext_flags |= IW_ENCODE_EXT_SET_TX_KEY;
 
 	ext->addr.sa_family = ARPHRD_ETHER;
@@ -1822,6 +1822,9 @@  static int wpa_driver_wext_set_key_ext(void *priv, enum wpa_alg alg,
  *	8-byte Rx Mic Key
  * @key_len: Length of the key buffer in octets (WEP: 5 or 13,
  *	TKIP: 32, CCMP: 16)
+ * @key_type: Additional key information. Only KEY_TYPE_DEFAULT is used
+ *	when the driver does not support separate unicast/individual key
+ *	to set the key as the default Tx key
  * Returns: 0 on success, -1 on failure
  *
  * This function uses SIOCSIWENCODEEXT by default, but tries to use
@@ -1837,9 +1840,9 @@  int wpa_driver_wext_set_key(const char *ifname, void *priv, enum wpa_alg alg,
 	struct iwreq iwr;
 	int ret = 0;
 
-	wpa_printf(MSG_DEBUG, "%s: alg=%d key_idx=%d set_tx=%d seq_len=%lu "
+	wpa_printf(MSG_DEBUG, "%s: alg=%d key_idx=%d key_type=%d seq_len=%lu "
 		   "key_len=%lu",
-		   __FUNCTION__, alg, key_idx, set_tx,
+		   __FUNCTION__, alg, key_idx, key_type,
 		   (unsigned long) seq_len, (unsigned long) key_len);
 
 	ret = wpa_driver_wext_set_key_ext(drv, alg, addr, key_idx, set_tx,
@@ -1873,7 +1876,7 @@  int wpa_driver_wext_set_key(const char *ifname, void *priv, enum wpa_alg alg,
 		ret = -1;
 	}
 
-	if (set_tx && alg != WPA_ALG_NONE) {
+	if (key_type == KEY_TYPE_DEFAULT && alg != WPA_ALG_NONE) {
 		os_memset(&iwr, 0, sizeof(iwr));
 		os_strlcpy(iwr.ifr_name, drv->ifname, IFNAMSIZ);
 		iwr.u.encoding.flags = key_idx + 1;