Migrate all drivers except nl80211 from set_tx to key_type. This patch assumes that the drivers only used set_tx for default keys as it was (probably) intended to be used and are not relaying on the quirks set_tx really has. The new KEY_TYPE_DEFAULT is only set for WEP default keys and when an AP is not using pairwise keys, handling all traffic only via a group key. Signed-off-by: Alexander Wetzel <alexander@xxxxxxxxxxxxxx> --- 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 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 also 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 set_tx int which is only used as boolean as a bit field to carry the additional information in other bits. But the sometimes incomprehensible usage of set_tx and the investigation how it should be done instead expanded the scope drastically. I believe now that migrating away from set_tx via multiple small patches is simpler to follow and review. 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 --git a/src/drivers/driver_atheros.c b/src/drivers/driver_atheros.c index 08095865a..0c87da6d0 100644 --- a/src/drivers/driver_atheros.c +++ b/src/drivers/driver_atheros.c @@ -569,7 +569,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); @@ -581,9 +581,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 c53155be0..89e4508e7 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 97c655521..290da9d5e 100644 --- a/src/drivers/driver_nl80211.c +++ b/src/drivers/driver_nl80211.c @@ -3484,7 +3484,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 ea5d667ed..52e8e8d49 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; @@ -1824,6 +1824,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 @@ -1839,9 +1842,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, @@ -1875,7 +1878,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; -- 2.23.0 _______________________________________________ Hostap mailing list Hostap@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/hostap
