Am 06.01.20 um 10:17 schrieb Jouni Malinen:
On Sat, Jan 04, 2020 at 11:10:04PM +0100, Alexander Wetzel wrote:
+enum key_flag {
+ KEY_FLAG_MODIFY = BIT(0),
+ KEY_FLAG_DEFAULT = BIT(1),
+ KEY_FLAG_RX = BIT(2),
+ KEY_FLAG_TX = BIT(3),
+ KEY_FLAG_GROUP = BIT(4),
+ KEY_FLAG_PAIRWISE = BIT(5),
+ KEY_FLAG_PMK = BIT(6),
How much of this is really needed for new functionality vs. cleaning up?
I'm not necessarily against these flags, but I'll note that the addr
parameter is already documented to indicate broadcast vs. default vs.
unicast keys.
I believe they are. And also that the documentation of addr should be
changed then, too:-)
The MODIFY Flag is not needed till we merge Extended Key ID (nobody is
changing a TX status so far). The PMK flag could also be called
something like "OTHER" or we also could just use "0" for them.
Just looking at the addr to figure out if we have a default key is
missing the (existing) special case of IBSS GTK keys. (Search for "RSN
IBSS RX GTK" in the driver) and look at supp_set_key() in ibss_rsn.c
which has this code block:
if (is_broadcast_ether_addr(addr))
addr = peer->addr;
return wpa_drv_set_key(peer->ibss_rsn->wpa_s, alg, addr, key_idx,
set_tx, seq, seq_len, key, key_len);
There also is the issue of the dual-usage of keyid 1 with Extended Key
ID when we delete a key. We have to figure out if the pairwise or the
group key must be deleted. (But just making sure pairwise keys are
flagged accordingly would sove that, it just would be a bit less obvious.)
That said we could achieve something similar with a slight update to the
key_flag API proposed in V8. But I really liked how the API proposed
here did clean up the special case handling in nl80211 and making that
much simpler.
diff --git a/src/drivers/driver.h b/src/drivers/driver.h
@@ -2359,7 +2400,7 @@ struct wpa_driver_ops {
int (*set_key)(const char *ifname, void *priv, enum wpa_alg alg,
const u8 *addr, int key_idx, int set_tx,
const u8 *seq, size_t seq_len,
- const u8 *key, size_t key_len);
+ const u8 *key, size_t key_len, enum key_flag key_flag);
Instead of doing these set_key() prototype changes in driver.h and all
driver_*.c in this patch 5/16 and then repeat that in 9/16, maybe it
would make more sense to have a new patch before 5/16 that converts
set_key() to take in a struct of parameter values similarly to
associate(). In other words, only take in void *priv and struct
wpa_driver_key_params *params. This change would only touch
src/drivers/* and the set_key() wrapper functions in
wpa_supplicant/driver_i.h and src/ap/ap_drv_ops.c. Addition of a new
key_flag into the struct could then be done without having to touch any
driver_*.c and similarly, removal of set_tx would be trivial once there
are no remaining users for it.
Ok, V10 of the series (or the three replacement series for it) will make
that change. (Will probably at least some days till I get to that.)
Alexander
_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap
