diff mbox series

[Regression] 4-way handshake offloading in wpa_supplicant 2.8

Message ID de043495-6bd1-422f-cae3-fbd8d7eb7bde@gmx.net
State Superseded
Headers show
Series [Regression] 4-way handshake offloading in wpa_supplicant 2.8 | expand

Commit Message

Stefan Wahren June 29, 2019, 9:37 a.m. UTC 
Hi,

i've found a regression with 4-way handshake offloading for 802.1X with wpa_supplicant 2.8. My setup consists of
Raspberry Pi 3 B (current linux-next, arm64/defconfig) on STA side and a Raspberry Pi 3 A+ (Linux 4.19) on AP side. The issue occurs on the STA side with wpa_supplicant 2.8, which gives the following output:

Configure PMK for driver-based RSN 4-way handshake
EAPOL: Successfully fetched key (len=32)
RSN: Configure PMK for driver-based 4-way handshake - hexdump(len=32):
[REMOVED]
wpa_driver_nl80211_set_key: ifindex=3 (wlan0) alg=5 addr=(nil) key_idx=0
set_tx=0 seq_len=0 key_len=32
nl80211: Set PMK to the driver for b8:27:eb:6c:5e:c9
nl80211: PMK - hexdump(len=32): [REMOVED]
nl80211: Set PMK failed: ret=-22 (Invalid argument)

During this the kernel also gave this warning:

[  874.485374] WARNING: CPU: 0 PID: 460 at
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:5208
brcmf_cfg80211_set_pmk+0x3c/0x58 [brcmfmac]
[  874.504523] Modules linked in: 8021q garp stp mrp llc bcm2835_v4l2(C)
brcmfmac vc4 v4l2_common videobuf2_vmalloc videobuf2_memops
videobuf2_v4l2 cec videobuf2_common drm_kms_helper videodev brcmutil
hci_uart cfg80211 mc btbcm drm snd_bcm2835(C) bluetooth smsc95xx
crct10dif_ce usbnet ecdh_generic ecc drm_panel_orientation_quirks
raspberrypi_hwmon rfkill bcm2835_rng bcm2835_thermal pwm_bcm2835
i2c_bcm2835 rng_core vchiq(C) ip_tables x_tables ipv6 nf_defrag_ipv6
[  874.558134] CPU: 0 PID: 460 Comm: wpa_supplicant Tainted: G       
WC        5.2.0-rc4-next-20190614-g65beedb66 #3
[  874.574984] Hardware name: Raspberry Pi 3 Model B (DT)
[  874.586546] pstate: 80000005 (Nzcv daif -PAN -UAO)
[  874.597817] pc : brcmf_cfg80211_set_pmk+0x3c/0x58 [brcmfmac]
[  874.610049] lr : nl80211_set_pmk+0x16c/0x1a8 [cfg80211]
[  874.621776] sp : ffff000011aab910
[  874.631533] x29: ffff000011aab910 x28: ffff80002ec5a000
[  874.643326] x27: 0000000000000014 x26: ffff80002fd9c300
[  874.655094] x25: ffff80002fd9c000 x24: ffff80002ec5c000
[  874.666843] x23: 00000000ffffff95 x22: ffff80002ec5d050
[  874.678580] x21: ffff80002ec5d008 x20: ffff000011aaba30
[  874.690336] x19: ffff000011349000 x18: 0000000000000000
[  874.702080] x17: 0000000000000000 x16: 0000000000000000
[  874.713809] x15: 0000000000000000 x14: be1127680d12277d
[  874.725547] x13: 8ba575fc53793d9f x12: ffff000008dff8a8
[  874.737297] x11: 0000000000000fe0 x10: 0000000000000000
[  874.749059] x9 : ffff000010c12068 x8 : ffff000010c12050
[  874.760832] x7 : ffff000008dfe8c8 x6 : 000000000000003f
[  874.772598] x5 : 0000000000000008 x4 : 000000006ceb27b8
[  874.784349] x3 : ffff000008ef1eb0 x2 : ffff000011aab978
[  874.796091] x1 : 0000000000000000 x0 : ffff80002ec5c7c0
[  874.807853] Call trace:
[  874.816698]  brcmf_cfg80211_set_pmk+0x3c/0x58 [brcmfmac]
[  874.828399]  nl80211_set_pmk+0x16c/0x1a8 [cfg80211]
[  874.839327]  genl_family_rcv_msg+0x364/0x460
[  874.849343]  genl_rcv_msg+0x5c/0xc0
[  874.858282]  netlink_rcv_skb+0x5c/0x128
[  874.867486]  genl_rcv+0x34/0x48
[  874.875956]  netlink_unicast+0x190/0x1f8
[  874.885203]  netlink_sendmsg+0x2cc/0x348
[  874.894397]  sock_sendmsg+0x18/0x30
[  874.903124]  ___sys_sendmsg+0x28c/0x2c8
[  874.912216]  __sys_sendmsg+0x6c/0xc8
[  874.921040]  __arm64_sys_sendmsg+0x20/0x28
[  874.930408]  el0_svc_common.constprop.0+0x64/0x160
[  874.940520]  el0_svc_handler+0x28/0x78
[  874.949552]  el0_svc+0x8/0xc
[  874.957674] ---[ end trace 72f634728d4e750f ]---

I already reported this to linux-wireless [1], but the driver maintainers said this is a regression in wpa_supplicant.
The actual STA configuration can be found here [2] and another report of this issue here [3].

[1] - https://marc.info/?l=linux-wireless&m=156061813109807&w=2
[2] - https://gist.github.com/lategoodbye/d4b5da60e905cbdf069affbd41cd14ab'
[3] - https://archlinuxarm.org/forum/viewtopic.php?f=60&t=13644

Here is the proposed fix for wpa_supplicant, which i successfully tested with 2.9-devel.

From 9774dfbf62f41080267ebb0943015a9f6d1dc0cf Mon Sep 17 00:00:00 2001
From: Chung-Hsien Hsu <stanley.hsu@cypress.com>
Date: Mon, 20 May 2019 17:10:39 +0800
Subject: [PATCH] wpa_supplicant: Fix 802.1X 4-way handshake offload indication

Commit d896874f8689 ("nl80211: Indicate 802.1X 4-way handshake
offload in connect") used the req_key_mgmt_offload flag to
indicate to the driver that it should offload the 802.1X handshake.
However, the flag will be updated according to th configuration of
proactive key caching and OKC if key management offload is considered
(it is enabled by default now). Do not update the flag if it has been
set for 802.1X 4-way handshake offload.

Signed-off-by: Chung-Hsien Hsu <stanley.hsu@cypress.com>
---
 wpa_supplicant/wpa_supplicant.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)


-- 2.1.0

Comments

Arend van Spriel July 1, 2019, 7:52 a.m. UTC | #1 
On 6/29/2019 11:37 AM, Stefan Wahren wrote:
> Hi,
> 
> i've found a regression with 4-way handshake offloading for 802.1X with wpa_supplicant 2.8. My setup consists of
> Raspberry Pi 3 B (current linux-next, arm64/defconfig) on STA side and a Raspberry Pi 3 A+ (Linux 4.19) on AP side. The issue occurs on the STA side with wpa_supplicant 2.8, which gives the following output:
> 
> Configure PMK for driver-based RSN 4-way handshake
> EAPOL: Successfully fetched key (len=32)
> RSN: Configure PMK for driver-based 4-way handshake - hexdump(len=32):
> [REMOVED]
> wpa_driver_nl80211_set_key: ifindex=3 (wlan0) alg=5 addr=(nil) key_idx=0
> set_tx=0 seq_len=0 key_len=32
> nl80211: Set PMK to the driver for b8:27:eb:6c:5e:c9
> nl80211: PMK - hexdump(len=32): [REMOVED]
> nl80211: Set PMK failed: ret=-22 (Invalid argument)
> 
> During this the kernel also gave this warning:
> 
> [  874.485374] WARNING: CPU: 0 PID: 460 at
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:5208
> brcmf_cfg80211_set_pmk+0x3c/0x58 [brcmfmac]
> [  874.504523] Modules linked in: 8021q garp stp mrp llc bcm2835_v4l2(C)
> brcmfmac vc4 v4l2_common videobuf2_vmalloc videobuf2_memops
> videobuf2_v4l2 cec videobuf2_common drm_kms_helper videodev brcmutil
> hci_uart cfg80211 mc btbcm drm snd_bcm2835(C) bluetooth smsc95xx
> crct10dif_ce usbnet ecdh_generic ecc drm_panel_orientation_quirks
> raspberrypi_hwmon rfkill bcm2835_rng bcm2835_thermal pwm_bcm2835
> i2c_bcm2835 rng_core vchiq(C) ip_tables x_tables ipv6 nf_defrag_ipv6
> [  874.558134] CPU: 0 PID: 460 Comm: wpa_supplicant Tainted: G
> WC        5.2.0-rc4-next-20190614-g65beedb66 #3
> [  874.574984] Hardware name: Raspberry Pi 3 Model B (DT)
> [  874.586546] pstate: 80000005 (Nzcv daif -PAN -UAO)
> [  874.597817] pc : brcmf_cfg80211_set_pmk+0x3c/0x58 [brcmfmac]
> [  874.610049] lr : nl80211_set_pmk+0x16c/0x1a8 [cfg80211]
> [  874.621776] sp : ffff000011aab910
> [  874.631533] x29: ffff000011aab910 x28: ffff80002ec5a000
> [  874.643326] x27: 0000000000000014 x26: ffff80002fd9c300
> [  874.655094] x25: ffff80002fd9c000 x24: ffff80002ec5c000
> [  874.666843] x23: 00000000ffffff95 x22: ffff80002ec5d050
> [  874.678580] x21: ffff80002ec5d008 x20: ffff000011aaba30
> [  874.690336] x19: ffff000011349000 x18: 0000000000000000
> [  874.702080] x17: 0000000000000000 x16: 0000000000000000
> [  874.713809] x15: 0000000000000000 x14: be1127680d12277d
> [  874.725547] x13: 8ba575fc53793d9f x12: ffff000008dff8a8
> [  874.737297] x11: 0000000000000fe0 x10: 0000000000000000
> [  874.749059] x9 : ffff000010c12068 x8 : ffff000010c12050
> [  874.760832] x7 : ffff000008dfe8c8 x6 : 000000000000003f
> [  874.772598] x5 : 0000000000000008 x4 : 000000006ceb27b8
> [  874.784349] x3 : ffff000008ef1eb0 x2 : ffff000011aab978
> [  874.796091] x1 : 0000000000000000 x0 : ffff80002ec5c7c0
> [  874.807853] Call trace:
> [  874.816698]  brcmf_cfg80211_set_pmk+0x3c/0x58 [brcmfmac]
> [  874.828399]  nl80211_set_pmk+0x16c/0x1a8 [cfg80211]
> [  874.839327]  genl_family_rcv_msg+0x364/0x460
> [  874.849343]  genl_rcv_msg+0x5c/0xc0
> [  874.858282]  netlink_rcv_skb+0x5c/0x128
> [  874.867486]  genl_rcv+0x34/0x48
> [  874.875956]  netlink_unicast+0x190/0x1f8
> [  874.885203]  netlink_sendmsg+0x2cc/0x348
> [  874.894397]  sock_sendmsg+0x18/0x30
> [  874.903124]  ___sys_sendmsg+0x28c/0x2c8
> [  874.912216]  __sys_sendmsg+0x6c/0xc8
> [  874.921040]  __arm64_sys_sendmsg+0x20/0x28
> [  874.930408]  el0_svc_common.constprop.0+0x64/0x160
> [  874.940520]  el0_svc_handler+0x28/0x78
> [  874.949552]  el0_svc+0x8/0xc
> [  874.957674] ---[ end trace 72f634728d4e750f ]---
> 
> I already reported this to linux-wireless [1], but the driver maintainers said this is a regression in wpa_supplicant.
> The actual STA configuration can be found here [2] and another report of this issue here [3].
> 
> [1] - https://marc.info/?l=linux-wireless&m=156061813109807&w=2
> [2] - https://gist.github.com/lategoodbye/d4b5da60e905cbdf069affbd41cd14ab'
> [3] - https://archlinuxarm.org/forum/viewtopic.php?f=60&t=13644
> 
> Here is the proposed fix for wpa_supplicant, which i successfully tested with 2.9-devel.

Hi Stefan,

I actually had another proposal, but was waiting for feedback from 
Jouni. So my idea was to introduce a separate flag for this offload as 
the req_key_mgmt_offload is for offload using different API, ie. a QCA 
vendor command. Below is the patch I prepared. Let me know if that works 
for you.

Regards,
Arend
---
diff --git a/src/drivers/driver.h b/src/drivers/driver.h
index d0f449f..16b438f 100644
--- a/src/drivers/driver.h
+++ b/src/drivers/driver.h
@@ -1067,6 +1067,14 @@ struct wpa_driver_associate_params {
  	int req_key_mgmt_offload;

  	/**
+	 * req_handshake_offload - Request EAPOL handshake offload
+	 *
+	 * Request EAPOL handshake offload for this connection if the device
+	 * supports it.
+	 */
+	int req_handshake_offload;
+
+	/**
  	 * Flag for indicating whether this association includes support for
  	 * RRM (Radio Resource Measurements)
  	 */
diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
index 45835a2..c44bd20 100644
--- a/src/drivers/driver_nl80211.c
+++ b/src/drivers/driver_nl80211.c
@@ -5632,7 +5632,7 @@ static int nl80211_connect_common(struct 
wpa_driver_nl80211_data *drv,
  			return -1;
  	}

-	if (params->req_key_mgmt_offload &&
+	if (params->req_handshake_offload &&
  	    (drv->capa.flags & WPA_DRIVER_FLAGS_4WAY_HANDSHAKE_8021X)) {
  		    wpa_printf(MSG_DEBUG, "  * WANT_1X_4WAY_HS");
  		    if (nla_put_flag(msg, NL80211_ATTR_WANT_1X_4WAY_HS))
diff --git a/wpa_supplicant/wpa_supplicant.c 
b/wpa_supplicant/wpa_supplicant.c
index 2002b12..0332d5a 100644
--- a/wpa_supplicant/wpa_supplicant.c
+++ b/wpa_supplicant/wpa_supplicant.c
@@ -3224,7 +3224,7 @@ static void wpas_start_assoc_cb(struct 
wpa_radio_work *work, int deinit)
  	     params.key_mgmt_suite == WPA_KEY_MGMT_IEEE8021X_SHA256 ||
  	     params.key_mgmt_suite == WPA_KEY_MGMT_IEEE8021X_SUITE_B ||
  	     params.key_mgmt_suite == WPA_KEY_MGMT_IEEE8021X_SUITE_B_192))
-		params.req_key_mgmt_offload = 1;
+		params.req_handshake_offload = 1;

  	if (wpa_s->conf->key_mgmt_offload) {
  		if (params.key_mgmt_suite == WPA_KEY_MGMT_IEEE8021X ||
Stefan Wahren July 1, 2019, 5:48 p.m. UTC | #2 
Hi Arend,

Am 01.07.19 um 09:52 schrieb Arend Van Spriel:
> On 6/29/2019 11:37 AM, Stefan Wahren wrote:
>
> ...
>
> Hi Stefan,
>
> I actually had another proposal, but was waiting for feedback from
> Jouni. So my idea was to introduce a separate flag for this offload as
> the req_key_mgmt_offload is for offload using different API, ie. a QCA
> vendor command. Below is the patch I prepared. Let me know if that
> works for you.
the patch below also fixes the issue. Thanks
>
> Regards,
> Arend
> ---
> diff --git a/src/drivers/driver.h b/src/drivers/driver.h
> index d0f449f..16b438f 100644
> --- a/src/drivers/driver.h
> +++ b/src/drivers/driver.h
> @@ -1067,6 +1067,14 @@ struct wpa_driver_associate_params {
>      int req_key_mgmt_offload;
>
>      /**
> +     * req_handshake_offload - Request EAPOL handshake offload
> +     *
> +     * Request EAPOL handshake offload for this connection if the device
> +     * supports it.
> +     */
> +    int req_handshake_offload;
> +
> +    /**
>       * Flag for indicating whether this association includes support for
>       * RRM (Radio Resource Measurements)
>       */
> diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
> index 45835a2..c44bd20 100644
> --- a/src/drivers/driver_nl80211.c
> +++ b/src/drivers/driver_nl80211.c
> @@ -5632,7 +5632,7 @@ static int nl80211_connect_common(struct
> wpa_driver_nl80211_data *drv,
>              return -1;
>      }
>
> -    if (params->req_key_mgmt_offload &&
> +    if (params->req_handshake_offload &&
>          (drv->capa.flags & WPA_DRIVER_FLAGS_4WAY_HANDSHAKE_8021X)) {
>              wpa_printf(MSG_DEBUG, "  * WANT_1X_4WAY_HS");
>              if (nla_put_flag(msg, NL80211_ATTR_WANT_1X_4WAY_HS))
> diff --git a/wpa_supplicant/wpa_supplicant.c
> b/wpa_supplicant/wpa_supplicant.c
> index 2002b12..0332d5a 100644
> --- a/wpa_supplicant/wpa_supplicant.c
> +++ b/wpa_supplicant/wpa_supplicant.c
> @@ -3224,7 +3224,7 @@ static void wpas_start_assoc_cb(struct
> wpa_radio_work *work, int deinit)
>           params.key_mgmt_suite == WPA_KEY_MGMT_IEEE8021X_SHA256 ||
>           params.key_mgmt_suite == WPA_KEY_MGMT_IEEE8021X_SUITE_B ||
>           params.key_mgmt_suite == WPA_KEY_MGMT_IEEE8021X_SUITE_B_192))
> -        params.req_key_mgmt_offload = 1;
> +        params.req_handshake_offload = 1;
>
>      if (wpa_s->conf->key_mgmt_offload) {
>          if (params.key_mgmt_suite == WPA_KEY_MGMT_IEEE8021X ||
Arend van Spriel July 1, 2019, 9:21 p.m. UTC | #3 
On 7/1/2019 7:48 PM, Stefan Wahren wrote:
> Hi Arend,
> 
> Am 01.07.19 um 09:52 schrieb Arend Van Spriel:
>> On 6/29/2019 11:37 AM, Stefan Wahren wrote:
>>
>> ...
>>
>> Hi Stefan,
>>
>> I actually had another proposal, but was waiting for feedback from
>> Jouni. So my idea was to introduce a separate flag for this offload as
>> the req_key_mgmt_offload is for offload using different API, ie. a QCA
>> vendor command. Below is the patch I prepared. Let me know if that
>> works for you.
> the patch below also fixes the issue. Thanks

Hi Stefan,

Thank you. I will submit it formally then.

Regards,
Arend
diff mbox series

Patch

diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
index 96a3691cf3cf..66ee268d861c 100644
--- a/wpa_supplicant/wpa_supplicant.c
+++ b/wpa_supplicant/wpa_supplicant.c
@@ -3221,8 +3221,10 @@  static void wpas_start_assoc_cb(struct wpa_radio_work *work, int deinit)
 	     params.key_mgmt_suite == WPA_KEY_MGMT_IEEE8021X_SUITE_B ||
 	     params.key_mgmt_suite == WPA_KEY_MGMT_IEEE8021X_SUITE_B_192))
 		params.req_key_mgmt_offload = 1;
+	else
+		params.req_key_mgmt_offload = 0;

-	if (wpa_s->conf->key_mgmt_offload) {
+	if (wpa_s->conf->key_mgmt_offload && !params.req_key_mgmt_offload) {
 		if (params.key_mgmt_suite == WPA_KEY_MGMT_IEEE8021X ||
 		    params.key_mgmt_suite == WPA_KEY_MGMT_IEEE8021X_SHA256 ||
 		    params.key_mgmt_suite == WPA_KEY_MGMT_IEEE8021X_SUITE_B ||