2022-10-05 13:06:07

by Johannes Berg

[permalink] [raw]
Subject: [PATCH 05/28] wifi: cfg80211: support reporting failed links

From: Johannes Berg <[email protected]>

For assoc and connect result APIs, support reporting
failed links; they should still come with the BSS
pointer in the case of assoc, so they're released
correctly. In the case of connect result, this is
optional.

Signed-off-by: Johannes Berg <[email protected]>
---
include/net/cfg80211.h | 7 +++++++
net/wireless/mlme.c | 4 ++++
net/wireless/nl80211.c | 5 ++++-
net/wireless/sme.c | 14 ++++++++++++++
4 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index e09ff87146c1..4d35a4234417 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -6933,6 +6933,8 @@ void cfg80211_auth_timeout(struct net_device *dev, const u8 *addr);
* @ap_mld_addr: AP MLD address (in case of MLO)
* @links: per-link information indexed by link ID, use links[0] for
* non-MLO connections
+ * @links.status: Set this (along with a BSS pointer) for links that
+ * were rejected by the AP.
*/
struct cfg80211_rx_assoc_resp {
const u8 *buf;
@@ -6944,6 +6946,7 @@ struct cfg80211_rx_assoc_resp {
struct {
const u8 *addr;
struct cfg80211_bss *bss;
+ u16 status;
} links[IEEE80211_MLD_MAX_NUM_LINKS];
};

@@ -7454,6 +7457,9 @@ struct cfg80211_fils_resp_params {
* if the bss is expired during the connection, esp. for those drivers
* implementing connect op. Only one parameter among @bssid and @bss needs
* to be specified.
+ * @links.status: per-link status code, to report a status code that's not
+ * %WLAN_STATUS_SUCCESS for a given link, it must also be in the
+ * @valid_links bitmap and may have a BSS pointer (which is then released)
*/
struct cfg80211_connect_resp_params {
int status;
@@ -7470,6 +7476,7 @@ struct cfg80211_connect_resp_params {
const u8 *addr;
const u8 *bssid;
struct cfg80211_bss *bss;
+ u16 status;
} links[IEEE80211_MLD_MAX_NUM_LINKS];
};

diff --git a/net/wireless/mlme.c b/net/wireless/mlme.c
index 581df7f4c524..58e1fb18f85a 100644
--- a/net/wireless/mlme.c
+++ b/net/wireless/mlme.c
@@ -42,6 +42,10 @@ void cfg80211_rx_assoc_resp(struct net_device *dev,
unsigned int link_id;

for (link_id = 0; link_id < ARRAY_SIZE(data->links); link_id++) {
+ cr.links[link_id].status = data->links[link_id].status;
+ WARN_ON_ONCE(cr.links[link_id].status != WLAN_STATUS_SUCCESS &&
+ (!cr.ap_mld_addr || !cr.links[link_id].bss));
+
cr.links[link_id].bss = data->links[link_id].bss;
if (!cr.links[link_id].bss)
continue;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 8ff8b1c040f0..ad7393cd3d18 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -17745,6 +17745,7 @@ void nl80211_send_connect_result(struct cfg80211_registered_device *rdev,
link_info_size += (cr->links[link].bssid ||
cr->links[link].bss) ?
nla_total_size(ETH_ALEN) : 0;
+ link_info_size += nla_total_size(sizeof(u16));
}
}

@@ -17813,7 +17814,9 @@ void nl80211_send_connect_result(struct cfg80211_registered_device *rdev,
nla_put(msg, NL80211_ATTR_BSSID, ETH_ALEN, bssid)) ||
(cr->links[link].addr &&
nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN,
- cr->links[link].addr)))
+ cr->links[link].addr)) ||
+ nla_put_u16(msg, NL80211_ATTR_STATUS_CODE,
+ cr->links[link].status))
goto nla_put_failure;

nla_nest_end(msg, nested_mlo_links);
diff --git a/net/wireless/sme.c b/net/wireless/sme.c
index d513536617bd..f94497e9db43 100644
--- a/net/wireless/sme.c
+++ b/net/wireless/sme.c
@@ -793,6 +793,10 @@ void __cfg80211_connect_result(struct net_device *dev,
}

for_each_valid_link(cr, link) {
+ /* don't do extra lookups for failures */
+ if (cr->links[link].status != WLAN_STATUS_SUCCESS)
+ continue;
+
if (cr->links[link].bss)
continue;

@@ -829,6 +833,16 @@ void __cfg80211_connect_result(struct net_device *dev,
}

memset(wdev->links, 0, sizeof(wdev->links));
+ for_each_valid_link(cr, link) {
+ if (cr->links[link].status == WLAN_STATUS_SUCCESS)
+ continue;
+ cr->valid_links &= ~BIT(link);
+ /* don't require bss pointer for failed links */
+ if (!cr->links[link].bss)
+ continue;
+ cfg80211_unhold_bss(bss_from_pub(cr->links[link].bss));
+ cfg80211_put_bss(wdev->wiphy, cr->links[link].bss);
+ }
wdev->valid_links = cr->valid_links;
for_each_valid_link(cr, link)
wdev->links[link].client.current_bss =
--
2.37.3


2022-10-06 10:51:40

by Veerendranath Jakkam

[permalink] [raw]
Subject: Re: [PATCH 05/28] wifi: cfg80211: support reporting failed links

I think similar enhancement needed for cfg80211_roamed/NL80211_CMD_ROAM
also to know accepted and rejected links info during roaming.

- veeru

2022-10-06 10:53:10

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 05/28] wifi: cfg80211: support reporting failed links

On Thu, 2022-10-06 at 16:19 +0530, Veerendranath Jakkam wrote:
> I think similar enhancement needed for cfg80211_roamed/NL80211_CMD_ROAM
> also to know accepted and rejected links info during roaming.
>

Not sure, in that case we might care only about the successful links
since wpa_s didn't request the others, so maybe it doesn't matter?

In any case, I have no driver I deal with that would ever use this API,
so I can't really do anything about it.

johannes

2022-10-06 11:11:15

by Veerendranath Jakkam

[permalink] [raw]
Subject: Re: [PATCH 05/28] wifi: cfg80211: support reporting failed links


On 10/6/2022 4:21 PM, Johannes Berg wrote:
> On Thu, 2022-10-06 at 16:19 +0530, Veerendranath Jakkam wrote:
>> I think similar enhancement needed for cfg80211_roamed/NL80211_CMD_ROAM
>> also to know accepted and rejected links info during roaming.
>>
> Not sure, in that case we might care only about the successful links
> since wpa_s didn't request the others, so maybe it doesn't matter?

Drivers can offload EAPOL 4WHS to wpa_supplicant during roaming with
non-FT AKMs. In that case supplicant needs to know rejected links info
to validate AP's MLO Link KDEs in 3/4 frame.

Ex: Driver handles only re-association request and response frames while
doing roaming with non-FT AKM then offload 4WHS to wpa_supplicant

>
> In any case, I have no driver I deal with that would ever use this API,
> so I can't really do anything about it.

No problem, I will post the required changes for
cfg80211_roamed/NL80211_CMD_ROAM path, hope that is fine.

>
> johannes

2022-10-06 11:12:12

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 05/28] wifi: cfg80211: support reporting failed links

On Thu, 2022-10-06 at 16:39 +0530, Veerendranath Jakkam wrote:
> On 10/6/2022 4:21 PM, Johannes Berg wrote:
> > On Thu, 2022-10-06 at 16:19 +0530, Veerendranath Jakkam wrote:
> > > I think similar enhancement needed for cfg80211_roamed/NL80211_CMD_ROAM
> > > also to know accepted and rejected links info during roaming.
> > >
> > Not sure, in that case we might care only about the successful links
> > since wpa_s didn't request the others, so maybe it doesn't matter?
>
> Drivers can offload EAPOL 4WHS to wpa_supplicant during roaming with
> non-FT AKMs. In that case supplicant needs to know rejected links info
> to validate AP's MLO Link KDEs in 3/4 frame.
>
> Ex: Driver handles only re-association request and response frames while
> doing roaming with non-FT AKM then offload 4WHS to wpa_supplicant
>

Right, but is there a fundamental difference between

"hey I roamed to this AP MLD with links 1, 2 and 5"
(with the right BSSIDs for the links etc.)

and

"hey I roamed to this AP MLD and I tried links 1, 2, 3, 4, 5 but only
1, 2 and 5 were established"

?

johannes

2022-10-06 14:42:49

by Veerendranath Jakkam

[permalink] [raw]
Subject: Re: [PATCH 05/28] wifi: cfg80211: support reporting failed links


On 10/6/2022 4:41 PM, Johannes Berg wrote:
> Right, but is there a fundamental difference between
> "hey I roamed to this AP MLD with links 1, 2 and 5"
> (with the right BSSIDs for the links etc.)
>
> and
>
> "hey I roamed to this AP MLD and I tried links 1, 2, 3, 4, 5 but only
> 1, 2 and 5 were established"
>
> ?

supplicant must validate MLO Link KDEs(includes RSNE and RSNXE) for all
the negotiated links but MLO GTK/IGTK/BIGTK only for accepted links
while processing 3/4 msg.

So, during roaming if EAPOL 4HS is offload to supplicant it should know
the requested and accepted links information.

non-AP MLD needs to select common AKM across all the links. The MLO Link
KDEs validation helps to avoid downgrade attacks.

- veeru