git: 2589197adb19 - main - net80211: migrate the group/unicast key check into inline functions
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Mon, 15 Jul 2024 18:46:37 UTC
The branch main has been updated by adrian:
URL: https://cgit.FreeBSD.org/src/commit/?id=2589197adb199ec37f132dd7e279eb0795713f1e
commit 2589197adb199ec37f132dd7e279eb0795713f1e
Author: Adrian Chadd <adrian@FreeBSD.org>
AuthorDate: 2024-06-06 17:28:03 +0000
Commit: Adrian Chadd <adrian@FreeBSD.org>
CommitDate: 2024-07-15 18:45:30 +0000
net80211: migrate the group/unicast key check into inline functions
The way that net80211 and drivers are checking for the /type/ of key
is to check if it's in the vap WEP key array and if so, it's a group
key. If not, it's a unicast key.
That's not only kind of terrible, but it's also going to be
problematic with future 802.11 support (for multiple unicast keys
and IGTK keys for management frame protection.)
So as part of this, remove the places where this is done and
instead use a pair inline functions - ieee80211_is_key_global() and
ieee80211_is_key_unicast(). They currenly still use the same logic
but the drivers and net80211 stack isn't doing it itself.
There are still open questions about why keys are not being
correctly tagged as GROUP, GTK, PTK, etc. That will be investigated
and addressed in follow-up work as a pre-cursor to MFP, IGTK, etc.
as mentioned above.
Testing:
* iwn, rtwn - STA mode
Differential Revision: https://reviews.freebsd.org/D45516
---
sys/dev/ath/if_ath_keycache.c | 3 +--
sys/dev/mwl/if_mwl.c | 3 +--
sys/dev/rtwn/if_rtwn_cam.c | 6 ++----
sys/dev/usb/wlan/if_rsu.c | 7 +++----
sys/dev/usb/wlan/if_rum.c | 10 ++++------
sys/dev/wpi/if_wpi.c | 11 +++++------
sys/net80211/ieee80211.c | 31 +++++++++++++++++++++++++++++++
sys/net80211/ieee80211_crypto.c | 16 ++++++++--------
sys/net80211/ieee80211_var.h | 5 +++++
9 files changed, 60 insertions(+), 32 deletions(-)
diff --git a/sys/dev/ath/if_ath_keycache.c b/sys/dev/ath/if_ath_keycache.c
index bc94273bf5ce..a58625ad2803 100644
--- a/sys/dev/ath/if_ath_keycache.c
+++ b/sys/dev/ath/if_ath_keycache.c
@@ -434,8 +434,7 @@ ath_key_alloc(struct ieee80211vap *vap, struct ieee80211_key *k,
/*
* Only global keys should have key index assigned.
*/
- if (!(&vap->iv_nw_keys[0] <= k &&
- k < &vap->iv_nw_keys[IEEE80211_WEP_NKID])) {
+ if (!ieee80211_is_key_global(vap, k)) {
/* should not happen */
DPRINTF(sc, ATH_DEBUG_KEYCACHE,
"%s: bogus group key\n", __func__);
diff --git a/sys/dev/mwl/if_mwl.c b/sys/dev/mwl/if_mwl.c
index 479f3144dce3..49b8b3279c7f 100644
--- a/sys/dev/mwl/if_mwl.c
+++ b/sys/dev/mwl/if_mwl.c
@@ -1519,8 +1519,7 @@ mwl_key_alloc(struct ieee80211vap *vap, struct ieee80211_key *k,
if (k->wk_keyix != IEEE80211_KEYIX_NONE ||
(k->wk_flags & IEEE80211_KEY_GROUP)) {
- if (!(&vap->iv_nw_keys[0] <= k &&
- k < &vap->iv_nw_keys[IEEE80211_WEP_NKID])) {
+ if (!ieee80211_is_key_global(vap, k)) {
/* should not happen */
DPRINTF(sc, MWL_DEBUG_KEYCACHE,
"%s: bogus group key\n", __func__);
diff --git a/sys/dev/rtwn/if_rtwn_cam.c b/sys/dev/rtwn/if_rtwn_cam.c
index 864c13d78285..d142cd0476e4 100644
--- a/sys/dev/rtwn/if_rtwn_cam.c
+++ b/sys/dev/rtwn/if_rtwn_cam.c
@@ -113,8 +113,7 @@ rtwn_key_alloc(struct ieee80211vap *vap, struct ieee80211_key *k,
struct rtwn_softc *sc = vap->iv_ic->ic_softc;
int i, start;
- if (&vap->iv_nw_keys[0] <= k &&
- k < &vap->iv_nw_keys[IEEE80211_WEP_NKID]) {
+ if (ieee80211_is_key_global(vap, k)) {
*keyix = ieee80211_crypto_get_key_wepidx(vap, k);
if (sc->sc_hwcrypto != RTWN_CRYPTO_FULL)
k->wk_flags |= IEEE80211_KEY_SWCRYPT;
@@ -308,8 +307,7 @@ rtwn_process_key(struct ieee80211vap *vap, const struct ieee80211_key *k,
return (1);
}
- if (&vap->iv_nw_keys[0] <= k &&
- k < &vap->iv_nw_keys[IEEE80211_WEP_NKID]) {
+ if (ieee80211_is_key_global(vap, k)) {
if (sc->sc_hwcrypto == RTWN_CRYPTO_FULL) {
struct rtwn_vap *rvp = RTWN_VAP(vap);
diff --git a/sys/dev/usb/wlan/if_rsu.c b/sys/dev/usb/wlan/if_rsu.c
index e000d1fb5992..c967435250ee 100644
--- a/sys/dev/usb/wlan/if_rsu.c
+++ b/sys/dev/usb/wlan/if_rsu.c
@@ -1526,10 +1526,10 @@ rsu_key_alloc(struct ieee80211vap *vap, struct ieee80211_key *k,
struct rsu_softc *sc = vap->iv_ic->ic_softc;
int is_checked = 0;
- if (&vap->iv_nw_keys[0] <= k &&
- k < &vap->iv_nw_keys[IEEE80211_WEP_NKID]) {
+ if (ieee80211_is_key_global(vap, k)) {
*keyix = ieee80211_crypto_get_key_wepidx(vap, k);
} else {
+ /* Note: assumes this is a pairwise key */
if (vap->iv_opmode != IEEE80211_M_STA) {
*keyix = 0;
/* TODO: obtain keyix from node id */
@@ -1570,8 +1570,7 @@ rsu_process_key(struct ieee80211vap *vap, const struct ieee80211_key *k,
}
/* Handle group keys. */
- if (&vap->iv_nw_keys[0] <= k &&
- k < &vap->iv_nw_keys[IEEE80211_WEP_NKID]) {
+ if (ieee80211_is_key_global(vap, k)) {
KASSERT(k->wk_keyix < nitems(sc->group_keys),
("keyix %u > %zu\n", k->wk_keyix, nitems(sc->group_keys)));
diff --git a/sys/dev/usb/wlan/if_rum.c b/sys/dev/usb/wlan/if_rum.c
index 2720f2ffedcb..edf92e2222b4 100644
--- a/sys/dev/usb/wlan/if_rum.c
+++ b/sys/dev/usb/wlan/if_rum.c
@@ -1468,8 +1468,7 @@ rum_tx_crypto_flags(struct rum_softc *sc, struct ieee80211_node *ni,
flags |= RT2573_TX_CIP_MODE(mode);
/* Do not trust GROUP flag */
- if (!(k >= &vap->iv_nw_keys[0] &&
- k < &vap->iv_nw_keys[IEEE80211_WEP_NKID]))
+ if (ieee80211_is_key_unicast(vap, k))
flags |= RT2573_TX_KEY_PAIR;
else
pos += 0 * RT2573_SKEY_MAX; /* vap id */
@@ -3006,8 +3005,7 @@ rum_key_alloc(struct ieee80211vap *vap, struct ieee80211_key *k,
struct rum_softc *sc = vap->iv_ic->ic_softc;
uint8_t i;
- if (!(&vap->iv_nw_keys[0] <= k &&
- k < &vap->iv_nw_keys[IEEE80211_WEP_NKID])) {
+ if (ieee80211_is_key_unicast(vap, k)) {
if (!(k->wk_flags & IEEE80211_KEY_SWCRYPT)) {
RUM_LOCK(sc);
for (i = 0; i < RT2573_ADDR_MAX; i++) {
@@ -3044,7 +3042,7 @@ rum_key_set(struct ieee80211vap *vap, const struct ieee80211_key *k)
return 1;
}
- group = k >= &vap->iv_nw_keys[0] && k < &vap->iv_nw_keys[IEEE80211_WEP_NKID];
+ group = ieee80211_is_key_global(vap, k);
return !rum_cmd_sleepable(sc, k, sizeof(*k), 0,
group ? rum_group_key_set_cb : rum_pair_key_set_cb);
@@ -3061,7 +3059,7 @@ rum_key_delete(struct ieee80211vap *vap, const struct ieee80211_key *k)
return 1;
}
- group = k >= &vap->iv_nw_keys[0] && k < &vap->iv_nw_keys[IEEE80211_WEP_NKID];
+ group = ieee80211_is_key_global(vap, k);
return !rum_cmd_sleepable(sc, k, sizeof(*k), 0,
group ? rum_group_key_del_cb : rum_pair_key_del_cb);
diff --git a/sys/dev/wpi/if_wpi.c b/sys/dev/wpi/if_wpi.c
index fc5cf02f3a25..11b8a749090d 100644
--- a/sys/dev/wpi/if_wpi.c
+++ b/sys/dev/wpi/if_wpi.c
@@ -4641,8 +4641,8 @@ again:
return !error;
}
- if (!(kflags & WPI_KFLAG_MULTICAST) && &vap->iv_nw_keys[0] <= k &&
- k < &vap->iv_nw_keys[IEEE80211_WEP_NKID]) {
+ if (!(kflags & WPI_KFLAG_MULTICAST) &&
+ ieee80211_is_key_global(vap, k)) {
kflags |= WPI_KFLAG_MULTICAST;
node.kflags = htole16(kflags);
@@ -4726,8 +4726,8 @@ again:
return !error;
}
- if (!(kflags & WPI_KFLAG_MULTICAST) && &vap->iv_nw_keys[0] <= k &&
- k < &vap->iv_nw_keys[IEEE80211_WEP_NKID]) {
+ if (!(kflags & WPI_KFLAG_MULTICAST) &&
+ ieee80211_is_key_global(vap, k)) {
kflags |= WPI_KFLAG_MULTICAST;
node.kflags = htole16(kflags);
@@ -4782,8 +4782,7 @@ wpi_process_key(struct ieee80211vap *vap, const struct ieee80211_key *k,
}
/* Handle group keys. */
- if (&vap->iv_nw_keys[0] <= k &&
- k < &vap->iv_nw_keys[IEEE80211_WEP_NKID]) {
+ if (ieee80211_is_key_global(vap, k)) {
WPI_NT_LOCK(sc);
if (set)
wvp->wv_gtk |= WPI_VAP_KEY(k->wk_keyix);
diff --git a/sys/net80211/ieee80211.c b/sys/net80211/ieee80211.c
index 9f91e31d13a6..ecf87020b066 100644
--- a/sys/net80211/ieee80211.c
+++ b/sys/net80211/ieee80211.c
@@ -2679,3 +2679,34 @@ ieee80211_channel_type_char(const struct ieee80211_channel *c)
return 'b';
return 'f';
}
+
+/*
+ * Determine whether the given key in the given VAP is a global key.
+ * (key index 0..3, shared between all stations on a VAP.)
+ *
+ * This is either a WEP key or a GROUP key.
+ *
+ * Note this will NOT return true if it is a IGTK key.
+ */
+bool
+ieee80211_is_key_global(const struct ieee80211vap *vap,
+ const struct ieee80211_key *key)
+{
+ return (&vap->iv_nw_keys[0] <= key &&
+ key < &vap->iv_nw_keys[IEEE80211_WEP_NKID]);
+}
+
+/*
+ * Determine whether the given key in the given VAP is a unicast key.
+ */
+bool
+ieee80211_is_key_unicast(const struct ieee80211vap *vap,
+ const struct ieee80211_key *key)
+{
+ /*
+ * This is a short-cut for now; eventually we will need
+ * to support multiple unicast keys, IGTK, etc) so we
+ * will absolutely need to fix the key flags.
+ */
+ return (!ieee80211_is_key_global(vap, key));
+}
diff --git a/sys/net80211/ieee80211_crypto.c b/sys/net80211/ieee80211_crypto.c
index 829653ff1335..d70b3aa4a24a 100644
--- a/sys/net80211/ieee80211_crypto.c
+++ b/sys/net80211/ieee80211_crypto.c
@@ -62,8 +62,8 @@ static int
null_key_alloc(struct ieee80211vap *vap, struct ieee80211_key *k,
ieee80211_keyix *keyix, ieee80211_keyix *rxkeyix)
{
- if (!(&vap->iv_nw_keys[0] <= k &&
- k < &vap->iv_nw_keys[IEEE80211_WEP_NKID])) {
+
+ if (!ieee80211_is_key_global(vap, k)) {
/*
* Not in the global key table, the driver should handle this
* by allocating a slot in the h/w key table/cache. In
@@ -606,9 +606,9 @@ ieee80211_crypto_get_key_wepidx(const struct ieee80211vap *vap,
const struct ieee80211_key *k)
{
- if (k >= &vap->iv_nw_keys[0] &&
- k < &vap->iv_nw_keys[IEEE80211_WEP_NKID])
+ if (ieee80211_is_key_global(vap, k)) {
return (k - vap->iv_nw_keys);
+ }
return (-1);
}
@@ -618,11 +618,11 @@ ieee80211_crypto_get_key_wepidx(const struct ieee80211vap *vap,
uint8_t
ieee80211_crypto_get_keyid(struct ieee80211vap *vap, struct ieee80211_key *k)
{
- if (k >= &vap->iv_nw_keys[0] &&
- k < &vap->iv_nw_keys[IEEE80211_WEP_NKID])
+ if (ieee80211_is_key_global(vap, k)) {
return (k - vap->iv_nw_keys);
- else
- return (0);
+ }
+
+ return (0);
}
struct ieee80211_key *
diff --git a/sys/net80211/ieee80211_var.h b/sys/net80211/ieee80211_var.h
index c827984b5f37..53139a3852b8 100644
--- a/sys/net80211/ieee80211_var.h
+++ b/sys/net80211/ieee80211_var.h
@@ -824,6 +824,11 @@ char ieee80211_channel_type_char(const struct ieee80211_channel *c);
#define ieee80211_get_home_channel(_ic) ((_ic)->ic_bsschan)
#define ieee80211_get_vap_desired_channel(_iv) ((_iv)->iv_des_chan)
+bool ieee80211_is_key_global(const struct ieee80211vap *vap,
+ const struct ieee80211_key *key);
+bool ieee80211_is_key_unicast(const struct ieee80211vap *vap,
+ const struct ieee80211_key *key);
+
void ieee80211_radiotap_attach(struct ieee80211com *,
struct ieee80211_radiotap_header *th, int tlen,
uint32_t tx_radiotap,