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
