| Message ID | 20190210210620.31181-3-alexander@wetzel-home.de (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | Draft for Extended Key ID support | expand |
On Sun, 2019-02-10 at 22:06 +0100, Alexander Wetzel wrote: > +/** > + * enum nl80211_key_install_mode - Key install mode > + * > + * @NL80211_KEY_RX_TX: Key must be installed for Rx and Tx > + * @NL80211_KEY_RX_ONLY: Allowed in combination with @NL80211_CMD_NEW_KEY: > + * Unicast key has to be installed for Rx only. > + * @NL80211_KEY_SWITCH_TX: Allowed in combination with @NL80211_CMD_SET_KEY: > + * Switch Tx to a Rx only, referenced by sta mac and idx. Don't you mean the other way around? Or, well, what you say is true for the *other* keys? > * by the %NL80211_SCAN_FLAG_MIN_PREQ_CONTENT flag. > * @NL80211_EXT_FEATURE_ENABLE_FTM_RESPONDER: Driver supports enabling fine > * timing measurement responder role. > - * no need to remove that :) > - /* only support setting default key */ > - if (!key.def && !key.defmgmt) > + /* Only support setting default key and > + * Extended Key ID action @NL80211_KEY_SWITCH_TX. > + */ you can remove the @, it's not a kernel-doc formatted comment > - } > + } else if (key.p.install_mode == NL80211_KEY_SWITCH_TX && > + wiphy_ext_feature_isset(&rdev->wiphy, > + NL80211_EXT_FEATURE_EXT_KEY_ID)) { > + u8 *mac_addr = NULL; > > + if (info->attrs[NL80211_ATTR_MAC]) > + mac_addr = nla_data(info->attrs[NL80211_ATTR_MAC]); > + > + if (!mac_addr || key.idx < 0 || key.idx > 1) { > + err = -EINVAL; > + goto out; > + } Really only 0 and 1 are allowed? Not 0-3? > +++ b/net/wireless/util.c > @@ -236,14 +236,22 @@ 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 > - * or a vendor specific cipher (because current deployments use > - * pairwise WEP keys with non-zero indices and for vendor > - * specific ciphers this should be validated in the driver or > - * hardware level - but 802.11i clearly specifies to use zero) > + /* IEEE802.11-2016 allows only 0 and - when using Extended Key > + * ID - 1 as index for pairwise keys. > + * @NL80211_KEY_RX_ONLY is only allowed for pairwise keys when > + * the driver supports Extended Key ID. > + * @NL80211_KEY_SWITCH_TX must not be set when validating a key. > */ > - if (pairwise && key_idx) > + if (params->install_mode == NL80211_KEY_RX_ONLY) { > + if (!wiphy_ext_feature_isset(&rdev->wiphy, > + NL80211_EXT_FEATURE_EXT_KEY_ID)) > + return -EINVAL; > + else if (!pairwise || key_idx < 0 || key_idx > 1) > + return -EINVAL; same question here johannes
>> +/** >> + * enum nl80211_key_install_mode - Key install mode >> + * >> + * @NL80211_KEY_RX_TX: Key must be installed for Rx and Tx >> + * @NL80211_KEY_RX_ONLY: Allowed in combination with @NL80211_CMD_NEW_KEY: >> + * Unicast key has to be installed for Rx only. >> + * @NL80211_KEY_SWITCH_TX: Allowed in combination with @NL80211_CMD_SET_KEY: >> + * Switch Tx to a Rx only, referenced by sta mac and idx. > > Don't you mean the other way around? Or, well, what you say is true for > the *other* keys? 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. "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. 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". NL80211_KEY_SWITCH_TX is only allowed with NL80211_CMD_SET_KEY and is the only Extended Key ID action allowed for that function. 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. 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. 2) Once ready will call NL80211_CMD_SET_KEY with flag NL80211_KEY_SWITCH_TX to activate a specific key. >> * by the %NL80211_SCAN_FLAG_MIN_PREQ_CONTENT flag. >> * @NL80211_EXT_FEATURE_ENABLE_FTM_RESPONDER: Driver supports enabling fine >> * timing measurement responder role. >> - * > > no need to remove that :) 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. > >> - /* only support setting default key */ >> - if (!key.def && !key.defmgmt) >> + /* Only support setting default key and >> + * Extended Key ID action @NL80211_KEY_SWITCH_TX. >> + */ > > you can remove the @, it's not a kernel-doc formatted comment > >> - } >> + } else if (key.p.install_mode == NL80211_KEY_SWITCH_TX && >> + wiphy_ext_feature_isset(&rdev->wiphy, >> + NL80211_EXT_FEATURE_EXT_KEY_ID)) { >> + u8 *mac_addr = NULL; >> >> + if (info->attrs[NL80211_ATTR_MAC]) >> + mac_addr = nla_data(info->attrs[NL80211_ATTR_MAC]); >> + >> + if (!mac_addr || key.idx < 0 || key.idx > 1) { >> + err = -EINVAL; >> + goto out; >> + } > > Really only 0 and 1 are allowed? Not 0-3? 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. Or also "12.7.6.4 4-way handshake message 2": If the Extended Key ID for Individually Addressed Frames subfield of the RSN Capabilities field is 1 for both the Authenticator and the Supplicant, then the Authenticator assigns a new Key ID for the PTKSA in the range 0 to 1 that is different from the Key ID assigned in the previous handshake and uses the MLME-SETKEYS.request primitive to install the new key to receive individually addressed MPDUs protected by the PTK with the assigned Key ID. > >> +++ b/net/wireless/util.c >> @@ -236,14 +236,22 @@ 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 >> - * or a vendor specific cipher (because current deployments use >> - * pairwise WEP keys with non-zero indices and for vendor >> - * specific ciphers this should be validated in the driver or >> - * hardware level - but 802.11i clearly specifies to use zero) >> + /* IEEE802.11-2016 allows only 0 and - when using Extended Key >> + * ID - 1 as index for pairwise keys. >> + * @NL80211_KEY_RX_ONLY is only allowed for pairwise keys when >> + * the driver supports Extended Key ID. >> + * @NL80211_KEY_SWITCH_TX must not be set when validating a key. >> */ >> - if (pairwise && key_idx) >> + if (params->install_mode == NL80211_KEY_RX_ONLY) { >> + if (!wiphy_ext_feature_isset(&rdev->wiphy, >> + NL80211_EXT_FEATURE_EXT_KEY_ID)) >> + return -EINVAL; >> + else if (!pairwise || key_idx < 0 || key_idx > 1) >> + return -EINVAL; > > same question here same answer with one remark: The original code did only allow id 0 and and I made sure only with install mode NL80211_KEY_RX_ONLY is allowed to use non-zero IDs. Alexander
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
Am 22.02.19 um 09:30 schrieb Johannes Berg: >> 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. "EXT" is only intended to be the short form for "Extended Key ID for Individually Addressed Frames" from IEEE 802.11 - 2016 and not as "extension for nl80211/mac80211". >> 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. > Ah, now I see your point! I'm viewing a Rx only key as something different, a new "key group" like the existing broadcast or unicast key groups. (Which btw. also why I have a problems to not have a key marked as such.) "Switch TX" was intended to bring across, that the Tx status would switch from the "other" key to the referenced one. (That's also the reason I did not want to use ENABLE_TX.) Or in other words: Make a potential existing RX/TX key to a RX only key and the current Rx only key to the RX/Tx key. The key referenced by sta and mac must be flagged Rx only or the command will have no effect/fail. (The last requirement would vanish when we drop the Rx_Only key flag.) > 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". What do you think about: Switch key referenced by referenced by sta mac and idx from RX- only to RX/TX and make any other RX/TX key for the sta a RX-only key. > >> 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". > See above. NL80211_KEY_ENABLE_TX would also implicit disable Tx for the "other" pairwise key (if present). Mac80211 e.g. evaluates delay tailromm for both keys again and always reduces it for the Rx only key when it was set and even sets the RX_only flag again. That said I'm happy with any of these: NL80211_KEY_ENABLE_TX NL80211_KEY_SWITCH_TX NL80211_KEY_MOVE_TX After that discussion I'm now wondering if we maybe should change the naming more fundamentally, similar to what I proposed for mac80211: NL80211_KEY_SET - install key and enable RX/TX (default) NL80211_KEY_EXT_SET add unicast key for Extended Key ID NL80211_KEY_EXT_ENABLE - Set the key referenced by sta mac the to the preferred TX key for sta Here it would be nice to be able to use "NL80211_EXT_KEY_", but that would break the naming schema quite badly. And using NL80211_KEY_EXT_KEY_ looks also confusing... Any preferences or other options? I think you have now all details about that and whatever looks best for you is it now. > Anyway, my whole comment was only about the documentation text which > seemed backward or at least unclear to me. > I did not see that when coming from understanding the code to writing comments/documentation. Chances are other readers will interpret that like you did and not as intended... And the area is complex enough without those misunderstandings. >> 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. > I understand. Honestly I would feel silly to put most of the drive-by changes into a dedicated patch and not worth the time for a review. So I guess I'll only do separate patches if I really care about the format/comment in future:-) Alexander
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index 7f2739a90bdb..71ebb3492e21 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -485,6 +485,7 @@ struct vif_params { * with the get_key() callback, must be in little endian, * length given by @seq_len. * @seq_len: length of @seq. + * @install_mode: key install mode (Rx/Tx, Rx only or set Tx) */ struct key_params { const u8 *key; @@ -492,6 +493,7 @@ struct key_params { int key_len; int seq_len; u32 cipher; + enum nl80211_key_install_mode install_mode; }; /** diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h index dd4f86ee286e..8ccede541913 100644 --- a/include/uapi/linux/nl80211.h +++ b/include/uapi/linux/nl80211.h @@ -4134,6 +4134,21 @@ enum nl80211_channel_type { NL80211_CHAN_HT40PLUS }; +/** + * enum nl80211_key_install_mode - Key install mode + * + * @NL80211_KEY_RX_TX: Key must be installed for Rx and Tx + * @NL80211_KEY_RX_ONLY: Allowed in combination with @NL80211_CMD_NEW_KEY: + * Unicast key has to be installed for Rx only. + * @NL80211_KEY_SWITCH_TX: Allowed in combination with @NL80211_CMD_SET_KEY: + * Switch Tx to a Rx only, referenced by sta mac and idx. + */ +enum nl80211_key_install_mode { + NL80211_KEY_RX_TX, + NL80211_KEY_RX_ONLY, + NL80211_KEY_SWITCH_TX +}; + /** * enum nl80211_chan_width - channel width definitions * @@ -4377,6 +4392,9 @@ enum nl80211_key_default_types { * @NL80211_KEY_DEFAULT_TYPES: A nested attribute containing flags * attributes, specifying what a key should be set as default as. * See &enum nl80211_key_default_types. + * @NL80211_KEY_INSTALL_MODE: the install mode from + * enum nl80211_key_install_mode. Defaults to @NL80211_KEY_RX_TX. + * * @__NL80211_KEY_AFTER_LAST: internal * @NL80211_KEY_MAX: highest key attribute */ @@ -4390,6 +4408,7 @@ enum nl80211_key_attributes { NL80211_KEY_DEFAULT_MGMT, NL80211_KEY_TYPE, NL80211_KEY_DEFAULT_TYPES, + NL80211_KEY_INSTALL_MODE, /* keep last */ __NL80211_KEY_AFTER_LAST, @@ -5330,11 +5349,12 @@ enum nl80211_feature_flags { * by the %NL80211_SCAN_FLAG_MIN_PREQ_CONTENT flag. * @NL80211_EXT_FEATURE_ENABLE_FTM_RESPONDER: Driver supports enabling fine * timing measurement responder role. - * * @NL80211_EXT_FEATURE_CAN_REPLACE_PTK0: Driver/device confirm that they are * able to rekey an in-use key correctly. Userspace must not rekey PTK keys * if this flag is not set. Ignoring this can leak clear text packets and/or * freeze the connection. + * @NL80211_EXT_FEATURE_EXT_KEY_ID: Driver supports "Extended Key ID for + * Individually Addressed Frames" from IEEE802.11-2016. * * @NL80211_EXT_FEATURE_AIRTIME_FAIRNESS: Driver supports getting airtime * fairness for transmitted packets and has enabled airtime fairness @@ -5384,6 +5404,7 @@ enum nl80211_ext_feature_index { NL80211_EXT_FEATURE_ENABLE_FTM_RESPONDER, NL80211_EXT_FEATURE_AIRTIME_FAIRNESS, NL80211_EXT_FEATURE_AP_PMKSA_CACHING, + NL80211_EXT_FEATURE_EXT_KEY_ID, /* add new features before the definition below */ NUM_NL80211_EXT_FEATURES, diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index a3cc039b9f55..2a076b99737e 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -565,6 +565,7 @@ static const struct nla_policy nl80211_key_policy[NL80211_KEY_MAX + 1] = { [NL80211_KEY_DEFAULT_MGMT] = { .type = NLA_FLAG }, [NL80211_KEY_TYPE] = NLA_POLICY_MAX(NLA_U32, NUM_NL80211_KEYTYPES - 1), [NL80211_KEY_DEFAULT_TYPES] = { .type = NLA_NESTED }, + [NL80211_KEY_INSTALL_MODE] = { .type = NLA_U8 }, }; /* policy for the key default flags */ @@ -970,6 +971,9 @@ static int nl80211_parse_key_new(struct genl_info *info, struct nlattr *key, k->def_multi = kdt[NL80211_KEY_DEFAULT_TYPE_MULTICAST]; } + if (tb[NL80211_KEY_INSTALL_MODE]) + k->p.install_mode = nla_get_u8(tb[NL80211_KEY_INSTALL_MODE]); + return 0; } @@ -3646,8 +3650,11 @@ static int nl80211_set_key(struct sk_buff *skb, struct genl_info *info) if (key.idx < 0) return -EINVAL; - /* only support setting default key */ - if (!key.def && !key.defmgmt) + /* Only support setting default key and + * Extended Key ID action @NL80211_KEY_SWITCH_TX. + */ + if (!key.def && !key.defmgmt && + !(key.p.install_mode == NL80211_KEY_SWITCH_TX)) return -EINVAL; wdev_lock(dev->ieee80211_ptr); @@ -3671,7 +3678,7 @@ static int nl80211_set_key(struct sk_buff *skb, struct genl_info *info) #ifdef CONFIG_CFG80211_WEXT dev->ieee80211_ptr->wext.default_key = key.idx; #endif - } else { + } else if (key.defmgmt) { if (key.def_uni || !key.def_multi) { err = -EINVAL; goto out; @@ -3693,8 +3700,25 @@ static int nl80211_set_key(struct sk_buff *skb, struct genl_info *info) #ifdef CONFIG_CFG80211_WEXT dev->ieee80211_ptr->wext.default_mgmt_key = key.idx; #endif - } + } else if (key.p.install_mode == NL80211_KEY_SWITCH_TX && + wiphy_ext_feature_isset(&rdev->wiphy, + NL80211_EXT_FEATURE_EXT_KEY_ID)) { + u8 *mac_addr = NULL; + if (info->attrs[NL80211_ATTR_MAC]) + mac_addr = nla_data(info->attrs[NL80211_ATTR_MAC]); + + if (!mac_addr || key.idx < 0 || key.idx > 1) { + err = -EINVAL; + goto out; + } + + err = rdev_add_key(rdev, dev, key.idx, + NL80211_KEYTYPE_PAIRWISE, + mac_addr, &key.p); + } else { + err = -EINVAL; + } out: wdev_unlock(dev->ieee80211_ptr); diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h index 5cb48d135fab..4bf4e3774825 100644 --- a/net/wireless/rdev-ops.h +++ b/net/wireless/rdev-ops.h @@ -77,7 +77,8 @@ static inline int rdev_add_key(struct cfg80211_registered_device *rdev, struct key_params *params) { int ret; - trace_rdev_add_key(&rdev->wiphy, netdev, key_index, pairwise, mac_addr); + trace_rdev_add_key(&rdev->wiphy, netdev, key_index, pairwise, + mac_addr, params->install_mode); ret = rdev->ops->add_key(&rdev->wiphy, netdev, key_index, pairwise, mac_addr, params); trace_rdev_return_int(&rdev->wiphy, ret); diff --git a/net/wireless/trace.h b/net/wireless/trace.h index 44b2ce1bb13a..b5c9e6729ff1 100644 --- a/net/wireless/trace.h +++ b/net/wireless/trace.h @@ -430,22 +430,43 @@ DECLARE_EVENT_CLASS(key_handle, BOOL_TO_STR(__entry->pairwise), MAC_PR_ARG(mac_addr)) ); -DEFINE_EVENT(key_handle, rdev_add_key, +DEFINE_EVENT(key_handle, rdev_get_key, TP_PROTO(struct wiphy *wiphy, struct net_device *netdev, u8 key_index, bool pairwise, const u8 *mac_addr), TP_ARGS(wiphy, netdev, key_index, pairwise, mac_addr) ); -DEFINE_EVENT(key_handle, rdev_get_key, +DEFINE_EVENT(key_handle, rdev_del_key, TP_PROTO(struct wiphy *wiphy, struct net_device *netdev, u8 key_index, bool pairwise, const u8 *mac_addr), TP_ARGS(wiphy, netdev, key_index, pairwise, mac_addr) ); -DEFINE_EVENT(key_handle, rdev_del_key, +TRACE_EVENT(rdev_add_key, TP_PROTO(struct wiphy *wiphy, struct net_device *netdev, u8 key_index, - bool pairwise, const u8 *mac_addr), - TP_ARGS(wiphy, netdev, key_index, pairwise, mac_addr) + bool pairwise, const u8 *mac_addr, u8 install_mode), + TP_ARGS(wiphy, netdev, key_index, pairwise, mac_addr, install_mode), + TP_STRUCT__entry( + WIPHY_ENTRY + NETDEV_ENTRY + MAC_ENTRY(mac_addr) + __field(u8, key_index) + __field(bool, pairwise) + __field(u8, install_mode) + ), + TP_fast_assign( + WIPHY_ASSIGN; + NETDEV_ASSIGN; + MAC_ASSIGN(mac_addr, mac_addr); + __entry->key_index = key_index; + __entry->pairwise = pairwise; + __entry->install_mode = install_mode; + ), + TP_printk(WIPHY_PR_FMT ", " NETDEV_PR_FMT ", key_index: %u, " + "install_mode: %u, pairwise: %s, mac addr: " MAC_PR_FMT, + WIPHY_PR_ARG, NETDEV_PR_ARG, __entry->key_index, + __entry->install_mode, BOOL_TO_STR(__entry->pairwise), + MAC_PR_ARG(mac_addr)) ); TRACE_EVENT(rdev_set_default_key, diff --git a/net/wireless/util.c b/net/wireless/util.c index cd48cdd582c0..a3338e611190 100644 --- a/net/wireless/util.c +++ b/net/wireless/util.c @@ -236,14 +236,22 @@ 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 - * or a vendor specific cipher (because current deployments use - * pairwise WEP keys with non-zero indices and for vendor - * specific ciphers this should be validated in the driver or - * hardware level - but 802.11i clearly specifies to use zero) + /* IEEE802.11-2016 allows only 0 and - when using Extended Key + * ID - 1 as index for pairwise keys. + * @NL80211_KEY_RX_ONLY is only allowed for pairwise keys when + * the driver supports Extended Key ID. + * @NL80211_KEY_SWITCH_TX must not be set when validating a key. */ - if (pairwise && key_idx) + if (params->install_mode == NL80211_KEY_RX_ONLY) { + if (!wiphy_ext_feature_isset(&rdev->wiphy, + NL80211_EXT_FEATURE_EXT_KEY_ID)) + return -EINVAL; + else if (!pairwise || key_idx < 0 || key_idx > 1) + return -EINVAL; + } else if ((pairwise && key_idx) || + params->install_mode == NL80211_KEY_SWITCH_TX) { return -EINVAL; + } break; case WLAN_CIPHER_SUITE_AES_CMAC: case WLAN_CIPHER_SUITE_BIP_CMAC_256:
Add support for IEEE 802.11-2016 "Extended Key ID for Individually Addressed Frames". Extends cfg80211 and nl80211 to allow pairwise keys to be installed Rx only, switch Tx over separately and to use keyidx 1 for pairwise keys. Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de> --- include/net/cfg80211.h | 2 ++ include/uapi/linux/nl80211.h | 23 ++++++++++++++++++++++- net/wireless/nl80211.c | 32 ++++++++++++++++++++++++++++---- net/wireless/rdev-ops.h | 3 ++- net/wireless/trace.h | 31 ++++++++++++++++++++++++++----- net/wireless/util.c | 20 ++++++++++++++------ 6 files changed, 94 insertions(+), 17 deletions(-)