On Wed, 2018-12-05 at 21:54 +0100, Alexander Wetzel wrote:
It started out as a flag and I switched to enum later without updating
it. I'll chnage that to nl80211_key_install_mode, much much better...
No, all stations using Extended Key ID will always use RX_ONLY and
SET_TX for pairwise key installs. The AP will install the Key Rx only
prior to sending eapol #3, the sta prior to sending eapol #4.
Actually ... let's see all the operations at nl80211 level.
We have NEW_KEY and SET_KEY, right? SET_KEY basically already says to
use a given key for TX from now on, IIRC?
So realistically, don't we only need
NEW_KEY (RX-only)
as a new variant of NEW_KEY?
Yes, that should indeed work. I'll try that and see how it plays out.
The PTK0 rekey patch added a new line in front of the description. The
next author did not notice that and added the description for
NL80211_EXT_FEATURE_ENABLE_FTM_RESPONDER at the wrong place, probably
assuming it was the end of the list. I've noticed that and fixed the
documentation order and the misleading empty line.
I'll break that out as a separate cleanup patch if nobody beats me to it:-)
Oh. OK. It also doesn't really matter ;-)
k->def_multi = true;
+ k->rx_only = !!info->attrs[NL80211_ATTR_KEY_RXONLY];
+ k->set_tx = !!info->attrs[NL80211_ATTR_KEY_SETTX];
shouldn't this already be install_mode?
Looks like switching to NL80211_ATTR_INSTALL_MODE will throw out that,
but with the code from this patch that's an interim layer for checks and
mapping it. So I'm not sure I understand that comment.
Well, me neither. Sounds almost like I got ahead of myself.
You need to check if extended key ID is supported
Yes. I have added checks in cfg80211_validate_key_settings current
development version already, making sure only valid combinations can be
called and reach this section.
Ok, great.
NL80211_CMD_SET_KEY is normally only used to set default keys for wep or
managment keys. That changes here.
Right.
In this API draft NL80211_CMD_NEW_KEY is only used when installing a
Extended Key ID key RX only. The switch to TX is added to
NL80211_CMD_SET_KEY. The code has some sanity checks and then tells the
driver to switch the key to tx.
Makes sense.
But that has been changed quite a bit, the procedure in this patch
turned out to be not so good and even had a locking issue, so it has
changed a bit. I guess we should shelf that till I get the new variant
working and then look at it again.
Fair enough.
+++ b/net/wireless/util.c
@@ -236,13 +236,14 @@ int cfg80211_validate_key_settings(struct cfg80211_registered_device *rdev,
case WLAN_CIPHER_SUITE_CCMP_256:
case WLAN_CIPHER_SUITE_GCMP:
case WLAN_CIPHER_SUITE_GCMP_256:
- /* Disallow pairwise keys with non-zero index unless it's WEP
+ /* IEEE802.11-2016 allows only 0 and 1 for pairwise keys.
+ * Disallow pairwise keys with index above 1 unless it's WEP
* or a vendor specific cipher (because current deployments use
- * pairwise WEP keys with non-zero indices and for vendor
+ * pairwise WEP keys with higher indices and for vendor
* specific ciphers this should be validated in the driver or
- * hardware level - but 802.11i clearly specifies to use zero)
+ * hardware level.
*/
- if (pairwise && key_idx)
+ if (pairwise && key_idx > 1)
return -EINVAL;
break;
Again, only if driver support is advertised, and the comment should
probably reference the feature bit from the spec.
That is where I added most of the sanity checks in the meantime.
But what feature bit from the spec are you referring to? The RSN
Capability one?
Well, I wasn't thinking that precisely. I just thought it should mention
that it now says "IEEE802.11-2016 allows only 0 and 1 for pairwise keys"
but doesn't clarify that this is only for stations that actually want to
support it, so it could be read as being always that way.
Makes sense. I'll reword that a bit.
