| Message ID | 1492808507-31224-1-git-send-email-arend.vanspriel@broadcom.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Johannes Berg |
| Headers | show |
On Fri, 2017-04-21 at 22:01 +0100, Arend van Spriel wrote: > I have been working on 4-way handshake offloading and one of the > things discussed was the addition of PORT_AUTHORIZED flag. So > this is what I came up with, but I suppose wpa_supplicant wants > to know whether it can expect this attribute or not. One option > is to have PORT_UNAUTHORIZED flag instead. Another option would > be introducing it as nl80211 protocol feature although not sure > if it could be considered as such. What do you guys think? I think it could be, but I'm not really sure it matters? > + (cr->port_state != CONTROL_PORT_STATE_UNAUTHORIZED && > + nla_put_flag(msg, NL80211_ATTR_PORT_AUTHORIZED)) || > (cr->req_ie && > This doesn't really make sense - why does unspecified equal authorized? johannes
On 25-4-2017 16:40, Johannes Berg wrote: > On Fri, 2017-04-21 at 22:01 +0100, Arend van Spriel wrote: > >> I have been working on 4-way handshake offloading and one of the >> things discussed was the addition of PORT_AUTHORIZED flag. So >> this is what I came up with, but I suppose wpa_supplicant wants >> to know whether it can expect this attribute or not. One option >> is to have PORT_UNAUTHORIZED flag instead. Another option would >> be introducing it as nl80211 protocol feature although not sure >> if it could be considered as such. What do you guys think? > > I think it could be, but I'm not really sure it matters? > >> + (cr->port_state != CONTROL_PORT_STATE_UNAUTHORIZED && >> + nla_put_flag(msg, NL80211_ATTR_PORT_AUTHORIZED)) || >> (cr->req_ie && >> > This doesn't really make sense - why does unspecified equal authorized? I was considering default behavior here for drivers that do not provide this information, ie. drivers not supporting 4-way handshake offload. So wpa_supplicant just looks for the PORT_AUTHORIZED attribute and deals with it without need for checking 4-way handshake offload is supported. Regards, Arend
On Tue, 2017-04-25 at 20:34 +0200, Arend Van Spriel wrote: > > > + (cr->port_state != CONTROL_PORT_STATE_UNAUTHORIZED > > > && > > > + nla_put_flag(msg, NL80211_ATTR_PORT_AUTHORIZED)) || > > > (cr->req_ie && > > > > > > > This doesn't really make sense - why does unspecified equal > > authorized? > > I was considering default behavior here for drivers that do not > provide this information, ie. drivers not supporting 4-way handshake > offload. So wpa_supplicant just looks for the PORT_AUTHORIZED > attribute and deals with it without need for checking 4-way handshake > offload is supported. There are two such cases - the driver is old and doesn't provide it, but of course once you see the attribute you know that's not the case. And the case that the driver doesn't support 4-way-HS. Can you really distinguish these though if you *don't* see the attribute? But anyway, if it's like mac80211 not providing the data at all, then it would say authorized, which seems wrong since that's clearly not the case for mac80211? Or maybe I'm just confused. johannes
On 25-4-2017 20:36, Johannes Berg wrote: > On Tue, 2017-04-25 at 20:34 +0200, Arend Van Spriel wrote: > >>>> + (cr->port_state != CONTROL_PORT_STATE_UNAUTHORIZED >>>> && >>>> + nla_put_flag(msg, NL80211_ATTR_PORT_AUTHORIZED)) || >>>> (cr->req_ie && >>>> >>> >>> This doesn't really make sense - why does unspecified equal >>> authorized? >> >> I was considering default behavior here for drivers that do not >> provide this information, ie. drivers not supporting 4-way handshake >> offload. So wpa_supplicant just looks for the PORT_AUTHORIZED >> attribute and deals with it without need for checking 4-way handshake >> offload is supported. > > There are two such cases - the driver is old and doesn't provide it, > but of course once you see the attribute you know that's not the case. > And the case that the driver doesn't support 4-way-HS. > > Can you really distinguish these though if you *don't* see the > attribute? > > But anyway, if it's like mac80211 not providing the data at all, then > it would say authorized, which seems wrong since that's clearly not the > case for mac80211? > > Or maybe I'm just confused. You might, but not about this ;-) The other approach I had in mind is to only pass the flag for drivers with 4-way-hs support. In that case wpa_supp also has to check that to determine whether the flag should be taken into account. Assuming the driver supporting 4-way-hs can provide the port state info. Otherwise, a new ext_feature flag would be needed. Regards, Arend
On Tue, 2017-04-25 at 20:56 +0200, Arend Van Spriel wrote: > > You might, but not about this ;-) The other approach I had in mind is > to only pass the flag for drivers with 4-way-hs support. In that case > wpa_supp also has to check that to determine whether the flag should > be taken into account. Assuming the driver supporting 4-way-hs can > provide the port state info. Otherwise, a new ext_feature flag would > be needed. I think it's reasonable to assume 4-way-HS offload drivers can support it. johannes
On 26-4-2017 9:20, Johannes Berg wrote: > On Tue, 2017-04-25 at 20:56 +0200, Arend Van Spriel wrote: >> >> You might, but not about this ;-) The other approach I had in mind is >> to only pass the flag for drivers with 4-way-hs support. In that case >> wpa_supp also has to check that to determine whether the flag should >> be taken into account. Assuming the driver supporting 4-way-hs can >> provide the port state info. Otherwise, a new ext_feature flag would >> be needed. > > I think it's reasonable to assume 4-way-HS offload drivers can support > it. I tested the 4-way-hs (both Personal and 802.1X) with boolean parameter similar to what is proposed in the patch for roaming "cfg80211/nl80211: add authorized flag to roaming event" and it works fine. So I can extend it for use in connect result. Just had one issue regarding the type, ie. flag vs. u8 because of how things are done in wpa_supplicant supporting QCA roam+auth vendor-specific event. Regards, Arend
> I tested the 4-way-hs (both Personal and 802.1X) with boolean > parameter similar to what is proposed in the patch for roaming > "cfg80211/nl80211: add authorized flag to roaming event" and it works > fine. So I can extend it for use in connect result. Just had one > issue regarding the type, ie. flag vs. u8 because of how things are > done in wpa_supplicant supporting QCA roam+auth vendor-specific > event. Ok. We have lots of related but somewhat conflicting things in flight right now, and we need somebody to go collect them all into a consistent patchset, preferably even with the 4-way-HS offload [1] thrown in. That needs rebasing anyway because it introduces the _PMK attribute that Jouni already added for FILS now. Do you want to pick that up? I probably can't do it soon - I'm likely busy with other things the next week or two, then travel again, ... johannes [1] https://patchwork.kernel.org/patch/9584201/
On 28-4-2017 14:02, Johannes Berg wrote: > >> I tested the 4-way-hs (both Personal and 802.1X) with boolean >> parameter similar to what is proposed in the patch for roaming >> "cfg80211/nl80211: add authorized flag to roaming event" and it works >> fine. So I can extend it for use in connect result. Just had one >> issue regarding the type, ie. flag vs. u8 because of how things are >> done in wpa_supplicant supporting QCA roam+auth vendor-specific >> event. > > Ok. > > We have lots of related but somewhat conflicting things in flight right > now, and we need somebody to go collect them all into a consistent > patchset, preferably even with the 4-way-HS offload [1] thrown in. That > needs rebasing anyway because it introduces the _PMK attribute that > Jouni already added for FILS now. I already have the 4-way-hs stuff rebased with modified ATTR_PMK description and additional length check in nl80211 for it. Jouni commented on that already in 4-way-hs patch. > Do you want to pick that up? I probably can't do it soon - I'm likely > busy with other things the next week or two, then travel again, ... That is my plan. Regards, Arend
On Fri, 2017-04-28 at 14:46 +0200, Arend Van Spriel wrote: > I already have the 4-way-hs stuff rebased with modified ATTR_PMK > description and additional length check in nl80211 for it. Jouni > commented on that already in 4-way-hs patch. Cool. > > Do you want to pick that up? I probably can't do it soon - I'm > > likely busy with other things the next week or two, then travel > > again, ... > > That is my plan. Awesome, thanks! johannes
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index bdc4424..f416d55 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -5236,6 +5236,12 @@ static inline void cfg80211_testmode_event(struct sk_buff *skb, gfp_t gfp) #define CFG80211_TESTMODE_DUMP(cmd) #endif +enum cfg80211_control_port_state { + CONTROL_PORT_STATE_UNSPECIFIED, + CONTROL_PORT_STATE_UNAUTHORIZED, + CONTROL_PORT_STATE_AUTHORIZED +}; + /** * struct cfg80211_connect_resp_params - Connection response params * @status: Status code, %WLAN_STATUS_SUCCESS for successful connection, use @@ -5271,6 +5277,8 @@ static inline void cfg80211_testmode_event(struct sk_buff *skb, gfp_t gfp) * not known. This value is used only if @status < 0 to indicate that the * failure is due to a timeout and not due to explicit rejection by the AP. * This value is ignored in other cases (@status >= 0). + * @port_state: Indicates whether the connection is ready to transport + * data packets (see &enum cfg80211_control_port_state). */ struct cfg80211_connect_resp_params { int status; @@ -5288,6 +5296,7 @@ struct cfg80211_connect_resp_params { size_t pmk_len; const u8 *pmkid; enum nl80211_timeout_reason timeout_reason; + enum cfg80211_control_port_state port_state; }; /** diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h index 087493d..34738df 100644 --- a/include/uapi/linux/nl80211.h +++ b/include/uapi/linux/nl80211.h @@ -2106,6 +2106,8 @@ enum nl80211_commands { * in %NL80211_CMD_CONNECT to indicate that for 802.1X authentication it * wants to use the supported offload. * @NL80211_ATTR_PMKR0_NAME: PMK-R0 Name for offloaded FT. + * @NL80211_ATTR_PORT_AUTHORIZED: flag attribute used in %NL80211_CMD_CONNECT + * notification indicating 4-way handshake offload finished successfully. * * @NUM_NL80211_ATTR: total number of nl80211_attrs available * @NL80211_ATTR_MAX: highest attribute number currently defined @@ -2531,6 +2533,7 @@ enum nl80211_attrs { NL80211_ATTR_WANT_1X_OFFLOAD, NL80211_ATTR_PMKR0_NAME, + NL80211_ATTR_PORT_AUTHORIZED, /* add attributes here, update the policy in nl80211.c */ diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index e08c0d3..7fff668 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -13745,6 +13745,8 @@ void nl80211_send_connect_result(struct cfg80211_registered_device *rdev, (nla_put_flag(msg, NL80211_ATTR_TIMED_OUT) || nla_put_u32(msg, NL80211_ATTR_TIMEOUT_REASON, cr->timeout_reason))) || + (cr->port_state != CONTROL_PORT_STATE_UNAUTHORIZED && + nla_put_flag(msg, NL80211_ATTR_PORT_AUTHORIZED)) || (cr->req_ie && nla_put(msg, NL80211_ATTR_REQ_IE, cr->req_ie_len, cr->req_ie)) || (cr->resp_ie && diff --git a/net/wireless/sme.c b/net/wireless/sme.c index 6459bb7..a0d4010 100644 --- a/net/wireless/sme.c +++ b/net/wireless/sme.c @@ -860,6 +860,7 @@ void cfg80211_connect_done(struct net_device *dev, ev->cr.bss = params->bss; ev->cr.status = params->status; ev->cr.timeout_reason = params->timeout_reason; + ev->cr.port_state = params->port_state; spin_lock_irqsave(&wdev->event_lock, flags); list_add_tail(&ev->list, &wdev->event_list);
When the driver supports offloading of the PTK/GTK handshakes completion of that during connect changes the layer 2 control port state to authorized. This patch allows the driver to pass that state in cfg80211_connect_done() resulting in adding the new flag NL80211_ATTR_PORT_AUTHORIZED in the NL80211_CMD_CONNECT notification. Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com> --- Hi Johannes, Jouni, et al I have been working on 4-way handshake offloading and one of the things discussed was the addition of PORT_AUTHORIZED flag. So this is what I came up with, but I suppose wpa_supplicant wants to know whether it can expect this attribute or not. One option is to have PORT_UNAUTHORIZED flag instead. Another option would be introducing it as nl80211 protocol feature although not sure if it could be considered as such. What do you guys think? Regards, Arend --- include/net/cfg80211.h | 9 +++++++++ include/uapi/linux/nl80211.h | 3 +++ net/wireless/nl80211.c | 2 ++ net/wireless/sme.c | 1 + 4 files changed, 15 insertions(+)