On Sun, 2019-02-17 at 20:19 +0100, Alexander Wetzel wrote: > 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. Right. > "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. Right. > 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". Ah. That's where the "EXT" came from in the mac80211 names :-) FWIW, I'm not sure that makes sense. Yes, we think of it as an "extension" or being "extended" now while we work on it, but ultimately it'll be a simple part of the API. But I digress. > NL80211_KEY_SWITCH_TX is only allowed with NL80211_CMD_SET_KEY and is > the only Extended Key ID action allowed for that function. Yes, but what does it *do*? Your documentation said: Switch Tx to a Rx only, referenced by sta mac and idx. but that seems backwards to me based on the above requirement: Allow a key to be used for decryption first and switch it to a "normal" Rx/Tx key with a second call. IOW, you said we need to have the ability to first install RX-only, and then switch the key to RX & TX. I'd have called this "ENABLE_TX" or something like that. Or perhaps SWITCH_(ON_)TX if you're so inclined, but the documentation should then say Switch key from RX-only to TX/RX, referenced by ... no? That's what I meant by "the other way around". > 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. Right. > 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. So far makes sense. > 2) Once ready will call NL80211_CMD_SET_KEY with flag > NL80211_KEY_SWITCH_TX to activate a specific key. Also makes sense, but the documentation doesn't. I think we should rename these perhaps NL80211_KEY_RX_TX - install key and enable RX/TX (default) NL80211_KEY_RX_ONLY - install unicast key for RX only NL80211_KEY_ENABLE_TX - enable TX for a previously installed RX_ONLY key I think? About the ENABLE_TX vs. SWITCH_TX - we don't allow to switch TX *off* again, only *on*, so I think "enable" makes more sense than "switch". Anyway, my whole comment was only about the documentation text which seemed backward or at least unclear to me. > 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. I have no objection to even the most trivial cleanup patches going in separately, and if you like multiple in a bigger "clean up this area" patch, but here I think it detracts from the patch. > 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. OK :) johannes
