supplicant: use separate flag for 4-way handshake offload
diff mbox series

Message ID 1562019229-15410-1-git-send-email-arend.vanspriel@broadcom.com
State Accepted
Headers show
Series
  • supplicant: use separate flag for 4-way handshake offload
Related show

Commit Message

Arend Van Spriel July 1, 2019, 10:13 p.m. UTC
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, this field was existing and used for a different offload
API. This causes wpa_supplicant to send a connect request with the
WANT_1X_HS flag and the subsequent set-pmk is rejected causing the
connection to fail. So this patch fixes that by introducing a new
flag req_handshake_offload so the offloads are no longer entangled.

Reported-by: Stefan Wahren <wahrenst@gmx.net>
Tested-by: Stefan Wahren <wahrenst@gmx.net>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
Hi Jouni,

I do not recall exactly what happened in commit d896874f8689. I suspect
I revised the patch using req_key_mgmt_offload without spinning another
test.

Regards,
Arend
---
 src/drivers/driver.h            | 8 ++++++++
 src/drivers/driver_nl80211.c    | 2 +-
 wpa_supplicant/wpa_supplicant.c | 2 +-
 3 files changed, 10 insertions(+), 2 deletions(-)

Comments

Arend Van Spriel July 11, 2019, 10:11 a.m. UTC | #1
On 7/2/2019 12:13 AM, Arend van Spriel wrote:
> 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, this field was existing and used for a different offload
> API. This causes wpa_supplicant to send a connect request with the

typo: should be "a connect request *without*". Should I resend or can 
this be fixed before applying?

Regards,
Arend
Stefan Wahren July 27, 2019, 10:08 p.m. UTC | #2
Am 02.07.19 um 00:13 schrieb Arend van Spriel:
> 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, this field was existing and used for a different offload
> API. This causes wpa_supplicant to send a connect request with the
> WANT_1X_HS flag and the subsequent set-pmk is rejected causing the
> connection to fail. So this patch fixes that by introducing a new
> flag req_handshake_offload so the offloads are no longer entangled.
>
> Reported-by: Stefan Wahren <wahrenst@gmx.net>
> Tested-by: Stefan Wahren <wahrenst@gmx.net>
> Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>

gentle ping
Jouni Malinen Aug. 1, 2019, 4:17 p.m. UTC | #3
On Tue, Jul 02, 2019 at 12:13:49AM +0200, Arend van Spriel wrote:
> 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, this field was existing and used for a different offload
> API. This causes wpa_supplicant to send a connect request with the
> WANT_1X_HS flag and the subsequent set-pmk is rejected causing the
> connection to fail. So this patch fixes that by introducing a new
> flag req_handshake_offload so the offloads are no longer entangled.

Thanks, applied.
Arend Van Spriel Aug. 2, 2019, 8:29 a.m. UTC | #4
On 8/1/2019 6:17 PM, Jouni Malinen wrote:
> On Tue, Jul 02, 2019 at 12:13:49AM +0200, Arend van Spriel wrote:
>> 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, this field was existing and used for a different offload
>> API. This causes wpa_supplicant to send a connect request with the

Don't know if a correction is possible, but reading back my commit 
message I noticed a typo. It should have been "connect request *without* 
WANT_1X_HS" here.

>> WANT_1X_HS flag and the subsequent set-pmk is rejected causing the
>> connection to fail. So this patch fixes that by introducing a new
>> flag req_handshake_offload so the offloads are no longer entangled.

Regards,
Arend
Jouni Malinen Aug. 3, 2019, 1:14 p.m. UTC | #5
On Fri, Aug 02, 2019 at 10:29:15AM +0200, Arend Van Spriel wrote:
> On 8/1/2019 6:17 PM, Jouni Malinen wrote:
> > On Tue, Jul 02, 2019 at 12:13:49AM +0200, Arend van Spriel wrote:
> > > 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, this field was existing and used for a different offload
> > > API. This causes wpa_supplicant to send a connect request with the
> 
> Don't know if a correction is possible, but reading back my commit message I
> noticed a typo. It should have been "connect request *without* WANT_1X_HS"
> here.

I thought I already fixed that in the commit. Anyway, if it is not
correct, the commit message cannot be changed after it has been pushed
to the master branch, i.e., the only alternative to changing something
would be to revert a commit and make a new one.
Arend Van Spriel Aug. 3, 2019, 2:19 p.m. UTC | #6
On August 3, 2019 3:14:51 PM Jouni Malinen <j@w1.fi> wrote:

> On Fri, Aug 02, 2019 at 10:29:15AM +0200, Arend Van Spriel wrote:
>> On 8/1/2019 6:17 PM, Jouni Malinen wrote:
>>> On Tue, Jul 02, 2019 at 12:13:49AM +0200, Arend van Spriel wrote:
>>>> 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, this field was existing and used for a different offload
>>>> API. This causes wpa_supplicant to send a connect request with the
>>
>> Don't know if a correction is possible, but reading back my commit message I
>> noticed a typo. It should have been "connect request *without* WANT_1X_HS"
>> here.
>
> I thought I already fixed that in the commit. Anyway, if it is not
> correct, the commit message cannot be changed after it has been pushed
> to the master branch, i.e., the only alternative to changing something
> would be to revert a commit and make a new one.

I checked the repo and you did correct it [1].

Thanks,
Arend

[1] https://w1.fi/cgit/hostap/commit/?id=cb28bd52e1f

Patch
diff mbox series

diff --git a/src/drivers/driver.h b/src/drivers/driver.h
index 8a5cdb8..2a8459a 100644
--- a/src/drivers/driver.h
+++ b/src/drivers/driver.h
@@ -1074,6 +1074,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 78eef38..911d79d 100644
--- a/wpa_supplicant/wpa_supplicant.c
+++ b/wpa_supplicant/wpa_supplicant.c
@@ -3229,7 +3229,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 ||