[PATCH] cfg80211: support 4-way handshake offloading for WPA/WPA2-PSK

[PATCH] cfg80211: support 4-way handshake offloading for WPA/WPA2-PSK

[Johannes Berg]

From: Eliad Peller <eliad@wizery.com>

Let drivers advertise support for station-mode 4-way handshake
offloading with a new NL80211_EXT_FEATURE_4WAY_HANDSHAKE_OFFLOAD_STA
flag.

Add a new NL80211_ATTR_PMK attribute that might be passed as part
of NL80211_CMD_CONNECT command, and contain the PSK (which is the
PMK, hence the name.)

The driver/device is assumed to handle the 4-way handshake by
itself in this case (including key derivations, etc.), instead
of relying on the supplicant.

This patch is somewhat based on this one (by Vladimir Kondratiev):
https://patchwork.kernel.org/patch/1309561/.

Signed-off-by: Vladimir Kondratiev <qca_vkondrat@qca.qualcomm.com>
Signed-off-by: Eliad Peller <eliadx.peller@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/linux/ieee80211.h    | 1 +
 include/net/cfg80211.h       | 2 ++
 include/uapi/linux/nl80211.h | 9 +++++++++
 net/wireless/nl80211.c       | 8 ++++++++
 4 files changed, 20 insertions(+)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 6ea381c98aae..2049e983a994 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -2357,6 +2357,7 @@ enum ieee80211_sa_query_action {
 #define WLAN_MAX_KEY_LEN		32
 
 #define WLAN_PMKID_LEN			16
+#define WLAN_PMK_LEN			32
 
 #define WLAN_OUI_WFA			0x506f9a
 #define WLAN_OUI_TYPE_WFA_P2P		9
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 86c12f85fb53..4ca968927d60 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -643,6 +643,7 @@ struct survey_info {
  * @wep_keys: static WEP keys, if not NULL points to an array of
  *	CFG80211_MAX_WEP_KEYS WEP keys
  * @wep_tx_key: key index (0..3) of the default TX static WEP key
+ * @psk: PSK (for devices supporting 4-way-handshake offload)
  */
 struct cfg80211_crypto_settings {
 	u32 wpa_versions;
@@ -656,6 +657,7 @@ struct cfg80211_crypto_settings {
 	bool control_port_no_encrypt;
 	struct key_params *wep_keys;
 	int wep_tx_key;
+	const u8 *psk;
 };
 
 /**
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 9a499b15cfbc..ba102c937750 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -2012,6 +2012,9 @@ enum nl80211_commands {
  *	u32 attribute with an &enum nl80211_timeout_reason value. This is used,
  *	e.g., with %NL80211_CMD_CONNECT event.
  *
+ * @NL80211_ATTR_PMK: PSK for offloaded 4-Way Handshake. Relevant only
+ *	with %NL80211_CMD_CONNECT (for WPA/WPA2-PSK networks).
+ *
  * @NUM_NL80211_ATTR: total number of nl80211_attrs available
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
@@ -2423,6 +2426,8 @@ enum nl80211_attrs {
 
 	NL80211_ATTR_TIMEOUT_REASON,
 
+	NL80211_ATTR_PMK,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
@@ -4759,6 +4764,9 @@ enum nl80211_feature_flags {
  * @NL80211_EXT_FEATURE_CQM_RSSI_LIST: With this driver the
  *	%NL80211_ATTR_CQM_RSSI_THOLD attribute accepts a list of zero or more
  *	RSSI threshold values to monitor rather than exactly one threshold.
+ * @NL80211_EXT_FEATURE_4WAY_HANDSHAKE_OFFLOAD_STA: Device supports
+ *	doing 4-way handshake in station mode (PSK is passed as part
+ *	of the connect command).
  *
  * @NUM_NL80211_EXT_FEATURES: number of extended features.
  * @MAX_NL80211_EXT_FEATURES: highest extended feature index.
@@ -4778,6 +4786,7 @@ enum nl80211_ext_feature_index {
 	NL80211_EXT_FEATURE_MGMT_TX_RANDOM_TA_CONNECTED,
 	NL80211_EXT_FEATURE_SCHED_SCAN_RELATIVE_RSSI,
 	NL80211_EXT_FEATURE_CQM_RSSI_LIST,
+	NL80211_EXT_FEATURE_4WAY_HANDSHAKE_OFFLOAD_STA,
 
 	/* add new features before the definition below */
 	NUM_NL80211_EXT_FEATURES,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index d516527fcb8e..34b551edbc4f 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -410,6 +410,7 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
 		.len = sizeof(struct nl80211_bss_select_rssi_adjust)
 	},
 	[NL80211_ATTR_TIMEOUT_REASON] = { .type = NLA_U32 },
+	[NL80211_ATTR_PMK] = { .len = WLAN_PMK_LEN },
 };
 
 /* policy for the key attributes */
@@ -8039,6 +8040,13 @@ static int nl80211_crypto_settings(struct cfg80211_registered_device *rdev,
 		memcpy(settings->akm_suites, data, len);
 	}
 
+	if (info->attrs[NL80211_ATTR_PMK]) {
+		if (!wiphy_ext_feature_isset(&rdev->wiphy,
+					     NL80211_EXT_FEATURE_4WAY_HANDSHAKE_OFFLOAD_STA))
+			return -EINVAL;
+		settings->psk = nla_data(info->attrs[NL80211_ATTR_PMK]);
+	}
+
 	return 0;
 }
 
-- 
2.9.3

Re: [PATCH] cfg80211: support 4-way handshake offloading for WPA/WPA2-PSK

[Arend Van Spriel]

On 21-2-2017 11:09, Johannes Berg wrote:
> From: Eliad Peller <eliad@wizery.com>
> 
> Let drivers advertise support for station-mode 4-way handshake
> offloading with a new NL80211_EXT_FEATURE_4WAY_HANDSHAKE_OFFLOAD_STA
> flag.

I find use of the term OFFLOAD a bit redundant as it is implied by its
presence anyway.

> Add a new NL80211_ATTR_PMK attribute that might be passed as part
> of NL80211_CMD_CONNECT command, and contain the PSK (which is the
> PMK, hence the name.)
> 
> The driver/device is assumed to handle the 4-way handshake by
> itself in this case (including key derivations, etc.), instead
> of relying on the supplicant.
> 
> This patch is somewhat based on this one (by Vladimir Kondratiev):
> https://patchwork.kernel.org/patch/1309561/.

It also looks quite like this one we posted a while ago [1]. Our bad
that we did not follow-up :-p

Thanks,
Arend

[1] https://www.spinics.net/lists/linux-wireless/msg129387.html

> Signed-off-by: Vladimir Kondratiev <qca_vkondrat@qca.qualcomm.com>
> Signed-off-by: Eliad Peller <eliadx.peller@intel.com>
> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
>  include/linux/ieee80211.h    | 1 +
>  include/net/cfg80211.h       | 2 ++
>  include/uapi/linux/nl80211.h | 9 +++++++++
>  net/wireless/nl80211.c       | 8 ++++++++
>  4 files changed, 20 insertions(+)
> 
> diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
> index 6ea381c98aae..2049e983a994 100644
> --- a/include/linux/ieee80211.h
> +++ b/include/linux/ieee80211.h
> @@ -2357,6 +2357,7 @@ enum ieee80211_sa_query_action {
>  #define WLAN_MAX_KEY_LEN		32
>  
>  #define WLAN_PMKID_LEN			16
> +#define WLAN_PMK_LEN			32
>  
>  #define WLAN_OUI_WFA			0x506f9a
>  #define WLAN_OUI_TYPE_WFA_P2P		9
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index 86c12f85fb53..4ca968927d60 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -643,6 +643,7 @@ struct survey_info {
>   * @wep_keys: static WEP keys, if not NULL points to an array of
>   *	CFG80211_MAX_WEP_KEYS WEP keys
>   * @wep_tx_key: key index (0..3) of the default TX static WEP key
> + * @psk: PSK (for devices supporting 4-way-handshake offload)
>   */
>  struct cfg80211_crypto_settings {
>  	u32 wpa_versions;
> @@ -656,6 +657,7 @@ struct cfg80211_crypto_settings {
>  	bool control_port_no_encrypt;
>  	struct key_params *wep_keys;
>  	int wep_tx_key;
> +	const u8 *psk;
>  };
>  
>  /**
> diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
> index 9a499b15cfbc..ba102c937750 100644
> --- a/include/uapi/linux/nl80211.h
> +++ b/include/uapi/linux/nl80211.h
> @@ -2012,6 +2012,9 @@ enum nl80211_commands {
>   *	u32 attribute with an &enum nl80211_timeout_reason value. This is used,
>   *	e.g., with %NL80211_CMD_CONNECT event.
>   *
> + * @NL80211_ATTR_PMK: PSK for offloaded 4-Way Handshake. Relevant only
> + *	with %NL80211_CMD_CONNECT (for WPA/WPA2-PSK networks).
> + *
>   * @NUM_NL80211_ATTR: total number of nl80211_attrs available
>   * @NL80211_ATTR_MAX: highest attribute number currently defined
>   * @__NL80211_ATTR_AFTER_LAST: internal use
> @@ -2423,6 +2426,8 @@ enum nl80211_attrs {
>  
>  	NL80211_ATTR_TIMEOUT_REASON,
>  
> +	NL80211_ATTR_PMK,
> +
>  	/* add attributes here, update the policy in nl80211.c */
>  
>  	__NL80211_ATTR_AFTER_LAST,
> @@ -4759,6 +4764,9 @@ enum nl80211_feature_flags {
>   * @NL80211_EXT_FEATURE_CQM_RSSI_LIST: With this driver the
>   *	%NL80211_ATTR_CQM_RSSI_THOLD attribute accepts a list of zero or more
>   *	RSSI threshold values to monitor rather than exactly one threshold.
> + * @NL80211_EXT_FEATURE_4WAY_HANDSHAKE_OFFLOAD_STA: Device supports
> + *	doing 4-way handshake in station mode (PSK is passed as part
> + *	of the connect command).
>   *
>   * @NUM_NL80211_EXT_FEATURES: number of extended features.
>   * @MAX_NL80211_EXT_FEATURES: highest extended feature index.
> @@ -4778,6 +4786,7 @@ enum nl80211_ext_feature_index {
>  	NL80211_EXT_FEATURE_MGMT_TX_RANDOM_TA_CONNECTED,
>  	NL80211_EXT_FEATURE_SCHED_SCAN_RELATIVE_RSSI,
>  	NL80211_EXT_FEATURE_CQM_RSSI_LIST,
> +	NL80211_EXT_FEATURE_4WAY_HANDSHAKE_OFFLOAD_STA,
>  
>  	/* add new features before the definition below */
>  	NUM_NL80211_EXT_FEATURES,
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index d516527fcb8e..34b551edbc4f 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -410,6 +410,7 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
>  		.len = sizeof(struct nl80211_bss_select_rssi_adjust)
>  	},
>  	[NL80211_ATTR_TIMEOUT_REASON] = { .type = NLA_U32 },
> +	[NL80211_ATTR_PMK] = { .len = WLAN_PMK_LEN },
>  };
>  
>  /* policy for the key attributes */
> @@ -8039,6 +8040,13 @@ static int nl80211_crypto_settings(struct cfg80211_registered_device *rdev,
>  		memcpy(settings->akm_suites, data, len);
>  	}
>  
> +	if (info->attrs[NL80211_ATTR_PMK]) {
> +		if (!wiphy_ext_feature_isset(&rdev->wiphy,
> +					     NL80211_EXT_FEATURE_4WAY_HANDSHAKE_OFFLOAD_STA))
> +			return -EINVAL;
> +		settings->psk = nla_data(info->attrs[NL80211_ATTR_PMK]);
> +	}
> +
>  	return 0;
>  }
>  
> 

Re: [PATCH] cfg80211: support 4-way handshake offloading for WPA/WPA2-PSK

[Johannes Berg]

On Tue, 2017-02-21 at 11:32 +0100, Arend Van Spriel wrote:
> On 21-2-2017 11:09, Johannes Berg wrote:
> > From: Eliad Peller <eliad@wizery.com>
> > 
> > Let drivers advertise support for station-mode 4-way handshake
> > offloading with a new
> > NL80211_EXT_FEATURE_4WAY_HANDSHAKE_OFFLOAD_STA
> > flag.
> 
> I find use of the term OFFLOAD a bit redundant as it is implied by
> its presence anyway.

Fair enough, we can remove that.

> > Add a new NL80211_ATTR_PMK attribute that might be passed as part
> > of NL80211_CMD_CONNECT command, and contain the PSK (which is the
> > PMK, hence the name.)
> > 
> > The driver/device is assumed to handle the 4-way handshake by
> > itself in this case (including key derivations, etc.), instead
> > of relying on the supplicant.
> > 
> > This patch is somewhat based on this one (by Vladimir Kondratiev):
> > https://patchwork.kernel.org/patch/1309561/.
> 
> It also looks quite like this one we posted a while ago [1]. Our bad
> that we did not follow-up :-p

Indeed, I was looking for the better link (had gmane recorded in the
internal commit log) and found a few that all looked the same ... :)

Would this work for you? We should have wpa_supplicant support too, but
need to ask Andrei to look at that.

johannes

Re: [PATCH] cfg80211: support 4-way handshake offloading for WPA/WPA2-PSK

[Arend Van Spriel]

On 21-2-2017 11:40, Johannes Berg wrote:
> On Tue, 2017-02-21 at 11:32 +0100, Arend Van Spriel wrote:
>> On 21-2-2017 11:09, Johannes Berg wrote:
>>> From: Eliad Peller <eliad@wizery.com>
>>>
>>> Let drivers advertise support for station-mode 4-way handshake
>>> offloading with a new
>>> NL80211_EXT_FEATURE_4WAY_HANDSHAKE_OFFLOAD_STA
>>> flag.
>>
>> I find use of the term OFFLOAD a bit redundant as it is implied by
>> its presence anyway.
> 
> Fair enough, we can remove that.
> 
>>> Add a new NL80211_ATTR_PMK attribute that might be passed as part
>>> of NL80211_CMD_CONNECT command, and contain the PSK (which is the
>>> PMK, hence the name.)

It is also added to NL80211_CMD_ASSOCIATE. Maybe worth mentioning.

>>> The driver/device is assumed to handle the 4-way handshake by
>>> itself in this case (including key derivations, etc.), instead
>>> of relying on the supplicant.

What I am also missing is how to deal with NL80211_CMD_CONNECTED event
signalling. Should that be given after (un)successful completion of the
4-way handshake or should we have a separate event for that?

Regards,
Arend

>>> This patch is somewhat based on this one (by Vladimir Kondratiev):
>>> https://patchwork.kernel.org/patch/1309561/.
>>
>> It also looks quite like this one we posted a while ago [1]. Our bad
>> that we did not follow-up :-p
> 
> Indeed, I was looking for the better link (had gmane recorded in the
> internal commit log) and found a few that all looked the same ... :)
> 
> Would this work for you? We should have wpa_supplicant support too, but
> need to ask Andrei to look at that.
> 
> johannes
> 

Re: [PATCH] cfg80211: support 4-way handshake offloading for WPA/WPA2-PSK

[Arend Van Spriel]

On 21-2-2017 12:34, Arend Van Spriel wrote:
> On 21-2-2017 11:40, Johannes Berg wrote:
>> On Tue, 2017-02-21 at 11:32 +0100, Arend Van Spriel wrote:
>>> On 21-2-2017 11:09, Johannes Berg wrote:
>>>> From: Eliad Peller <eliad@wizery.com>
>>>>
>>>> Let drivers advertise support for station-mode 4-way handshake
>>>> offloading with a new
>>>> NL80211_EXT_FEATURE_4WAY_HANDSHAKE_OFFLOAD_STA
>>>> flag.
>>>
>>> I find use of the term OFFLOAD a bit redundant as it is implied by
>>> its presence anyway.
>>
>> Fair enough, we can remove that.
>>
>>>> Add a new NL80211_ATTR_PMK attribute that might be passed as part
>>>> of NL80211_CMD_CONNECT command, and contain the PSK (which is the
>>>> PMK, hence the name.)
> 
> It is also added to NL80211_CMD_ASSOCIATE. Maybe worth mentioning.
> 
>>>> The driver/device is assumed to handle the 4-way handshake by
>>>> itself in this case (including key derivations, etc.), instead
>>>> of relying on the supplicant.
> 
> What I am also missing is how to deal with NL80211_CMD_CONNECTED event
> signalling. Should that be given after (un)successful completion of the
> 4-way handshake or should we have a separate event for that?
> 
> Regards,
> Arend
> 
>>>> This patch is somewhat based on this one (by Vladimir Kondratiev):
>>>> https://patchwork.kernel.org/patch/1309561/.
>>>
>>> It also looks quite like this one we posted a while ago [1]. Our bad
>>> that we did not follow-up :-p
>>
>> Indeed, I was looking for the better link (had gmane recorded in the
>> internal commit log) and found a few that all looked the same ... :)
>>
>> Would this work for you? We should have wpa_supplicant support too, but
>> need to ask Andrei to look at that.

Forgot to reply to this. Yes, this will work. Can come up with wpa_supp
changes.

Regards,
Arend

Re: [PATCH] cfg80211: support 4-way handshake offloading for WPA/WPA2-PSK

[Johannes Berg]

> > > Would this work for you? We should have wpa_supplicant support
> > > too, but need to ask Andrei to look at that.
> 
> Forgot to reply to this. Yes, this will work. Can come up with
> wpa_supp changes.

No, we have them. Just need to see where they are :)

johannes

Re: [PATCH] cfg80211: support 4-way handshake offloading for WPA/WPA2-PSK

[Arend Van Spriel]

On 21-2-2017 12:46, Johannes Berg wrote:
> 
>>>> Would this work for you? We should have wpa_supplicant support
>>>> too, but need to ask Andrei to look at that.
>>
>> Forgot to reply to this. Yes, this will work. Can come up with
>> wpa_supp changes.
> 
> No, we have them. Just need to see where they are :)

I see. Will keep an eye on hostapd list ;-)

Gr. AvS

Re: [PATCH] cfg80211: support 4-way handshake offloading for WPA/WPA2-PSK

[Johannes Berg]

> What I am also missing is how to deal with NL80211_CMD_CONNECTED
> event signalling. Should that be given after (un)successful
> completion of the 4-way handshake or should we have a separate event
> for that? 

That's a good point. I don't think a new event really works well, but
perhaps we should extend the NL80211_TIMEOUT_* values with this? It's
not strictly a timeout, I guess, but it's not a direct rejection of the
association either.

Any thoughts?

johannes

Re: [PATCH] cfg80211: support 4-way handshake offloading for WPA/WPA2-PSK

[Arend Van Spriel]

On 21-2-2017 13:37, Johannes Berg wrote:
> 
>> What I am also missing is how to deal with NL80211_CMD_CONNECTED
>> event signalling. Should that be given after (un)successful
>> completion of the 4-way handshake or should we have a separate event
>> for that? 
> 
> That's a good point. I don't think a new event really works well, but
> perhaps we should extend the NL80211_TIMEOUT_* values with this? It's
> not strictly a timeout, I guess, but it's not a direct rejection of the
> association either.
> 
> Any thoughts?

I discussed about the event mechanism with Jithu which is who asked me
to look at their patches for this feature. His suggestion was to have a
"port authorized" event after successful handshake as it will help in
key mgmt offload cases. He gave the following example: Suppose you are
doing a fresh 8021x followed by 4way handshake offload. Right now there
is no event to indicate to the upper layer that the 4way handshake have
been completed successfully. So the only thing to do in wpa_supplicant
is to consider it completed and if not be hit with deauth.

Regards,
Arend

Re: [PATCH] cfg80211: support 4-way handshake offloading for WPA/WPA2-PSK

[Jithu Jance]

Hi Jouni, Johannes et al.,

On Wed, Feb 22, 2017 at 1:27 AM, Arend Van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 21-2-2017 13:37, Johannes Berg wrote:
>>
>>> What I am also missing is how to deal with NL80211_CMD_CONNECTED
>>> event signalling. Should that be given after (un)successful
>>> completion of the 4-way handshake or should we have a separate event
>>> for that?
>>
>> That's a good point. I don't think a new event really works well, but
>> perhaps we should extend the NL80211_TIMEOUT_* values with this? It's
>> not strictly a timeout, I guess, but it's not a direct rejection of the
>> association either.
>>
>> Any thoughts?
>
> I discussed about the event mechanism with Jithu which is who asked me
> to look at their patches for this feature. His suggestion was to have a
> "port authorized" event after successful handshake as it will help in
> key mgmt offload cases. He gave the following example: Suppose you are
> doing a fresh 8021x followed by 4way handshake offload. Right now there
> is no event to indicate to the upper layer that the 4way handshake have
> been completed successfully. So the only thing to do in wpa_supplicant
> is to consider it completed and if not be hit with deauth.

In wpa_supplicant terms, I was looking for an event notification to move the
wpa_state from WPA_ASSOCIATED to WPA_COMPLETED.  :)

Thanks,

-- 

Jithu Jance

Re: [PATCH] cfg80211: support 4-way handshake offloading for WPA/WPA2-PSK

[Arend Van Spriel]

On 22-2-2017 13:24, Jithu Jance wrote:
> Hi Jouni, Johannes et al.,
> 
> On Wed, Feb 22, 2017 at 1:27 AM, Arend Van Spriel
> <arend.vanspriel@broadcom.com> wrote:
>> On 21-2-2017 13:37, Johannes Berg wrote:
>>>
>>>> What I am also missing is how to deal with NL80211_CMD_CONNECTED
>>>> event signalling. Should that be given after (un)successful
>>>> completion of the 4-way handshake or should we have a separate event
>>>> for that?
>>>
>>> That's a good point. I don't think a new event really works well, but
>>> perhaps we should extend the NL80211_TIMEOUT_* values with this? It's
>>> not strictly a timeout, I guess, but it's not a direct rejection of the
>>> association either.
>>>
>>> Any thoughts?
>>
>> I discussed about the event mechanism with Jithu which is who asked me
>> to look at their patches for this feature. His suggestion was to have a
>> "port authorized" event after successful handshake as it will help in
>> key mgmt offload cases. He gave the following example: Suppose you are
>> doing a fresh 8021x followed by 4way handshake offload. Right now there
>> is no event to indicate to the upper layer that the 4way handshake have
>> been completed successfully. So the only thing to do in wpa_supplicant
>> is to consider it completed and if not be hit with deauth.
> 
> In wpa_supplicant terms, I was looking for an event notification to move the
> wpa_state from WPA_ASSOCIATED to WPA_COMPLETED.  :)

Currently in wpa_supplicant_event_assoc() the 4-way handshake offload is
handled as below:

        if ((wpa_s->drv_flags & WPA_DRIVER_FLAGS_4WAY_HANDSHAKE) &&
            wpa_key_mgmt_wpa_psk(wpa_s->key_mgmt)) {
                /*
                 * We are done; the driver will take care of RSN 4-way
                 * handshake.
                 */
                wpa_supplicant_cancel_auth_timeout(wpa_s);
                wpa_supplicant_set_state(wpa_s, WPA_COMPLETED);
                eapol_sm_notify_portValid(wpa_s->eapol, TRUE);
                eapol_sm_notify_eap_success(wpa_s->eapol, TRUE);
        } else if ((wpa_s->drv_flags & WPA_DRIVER_FLAGS_4WAY_HANDSHAKE) &&
                   wpa_key_mgmt_wpa_ieee8021x(wpa_s->key_mgmt)) {
                /*
                 * The driver will take care of RSN 4-way handshake, so
we need
                 * to allow EAPOL supplicant to complete its work without
                 * waiting for WPA supplicant.
                 */
                eapol_sm_notify_portValid(wpa_s->eapol, TRUE);
        } else if (ft_completed) {

So for 4-way hs offload for wpa/wpa2-psk it already calls
eapol_sm_notify_eap_success() and reading the documentation of that
function:

/**
 * eapol_sm_notify_eap_success - Notification of external EAP success
trigger
 * @sm: Pointer to EAPOL state machine allocated with eapol_sm_init()
 * @success: %TRUE = set success, %FALSE = clear success
 *
 * Notify the EAPOL state machine that external event has forced EAP
state to
 * success (success = %TRUE). This can be cleared by setting success =
%FALSE.
 *
 * This function is called to update EAP state when WPA-PSK key
handshake has
 * been completed successfully since WPA-PSK does not use EAP state machine.
 */
void eapol_sm_notify_eap_success(struct eapol_sm *sm, Boolean success)

So the code in wpa_supplicant_event_assoc() is either lying or the assoc
event should come after driver has completed successfully. To me it
seems worth it to have NL80211_CMD_EAP_COMPLETED event carrying the
result or indeed give the CONNECTED event later in the driver.

Regards,
Arend

Re: [PATCH] cfg80211: support 4-way handshake offloading for WPA/WPA2-PSK

[Johannes Berg]

> So the code in wpa_supplicant_event_assoc() is either lying or the
> assoc event should come after driver has completed successfully. To
> me it seems worth it to have NL80211_CMD_EAP_COMPLETED event carrying
> the result or indeed give the CONNECTED event later in the driver.

Sorry - need to look at this in more detail later.

But yes, we do give CONNECTED event later in the driver right now.

johannes

Re: [PATCH] cfg80211: support 4-way handshake offloading for WPA/WPA2-PSK

[Arend Van Spriel]

On 23-2-2017 11:36, Arend Van Spriel wrote:
> On 22-2-2017 13:24, Jithu Jance wrote:
>> Hi Jouni, Johannes et al.,
>>
>> On Wed, Feb 22, 2017 at 1:27 AM, Arend Van Spriel
>> <arend.vanspriel@broadcom.com> wrote:
>>> On 21-2-2017 13:37, Johannes Berg wrote:
>>>>
>>>>> What I am also missing is how to deal with NL80211_CMD_CONNECTED
>>>>> event signalling. Should that be given after (un)successful
>>>>> completion of the 4-way handshake or should we have a separate event
>>>>> for that?
>>>>
>>>> That's a good point. I don't think a new event really works well, but
>>>> perhaps we should extend the NL80211_TIMEOUT_* values with this? It's
>>>> not strictly a timeout, I guess, but it's not a direct rejection of the
>>>> association either.
>>>>
>>>> Any thoughts?
>>>
>>> I discussed about the event mechanism with Jithu which is who asked me
>>> to look at their patches for this feature. His suggestion was to have a
>>> "port authorized" event after successful handshake as it will help in
>>> key mgmt offload cases. He gave the following example: Suppose you are
>>> doing a fresh 8021x followed by 4way handshake offload. Right now there
>>> is no event to indicate to the upper layer that the 4way handshake have
>>> been completed successfully. So the only thing to do in wpa_supplicant
>>> is to consider it completed and if not be hit with deauth.
>>
>> In wpa_supplicant terms, I was looking for an event notification to move the
>> wpa_state from WPA_ASSOCIATED to WPA_COMPLETED.  :)
> 
> Currently in wpa_supplicant_event_assoc() the 4-way handshake offload is
> handled as below:
> 
>         if ((wpa_s->drv_flags & WPA_DRIVER_FLAGS_4WAY_HANDSHAKE) &&
>             wpa_key_mgmt_wpa_psk(wpa_s->key_mgmt)) {
>                 /*
>                  * We are done; the driver will take care of RSN 4-way
>                  * handshake.
>                  */
>                 wpa_supplicant_cancel_auth_timeout(wpa_s);
>                 wpa_supplicant_set_state(wpa_s, WPA_COMPLETED);
>                 eapol_sm_notify_portValid(wpa_s->eapol, TRUE);
>                 eapol_sm_notify_eap_success(wpa_s->eapol, TRUE);
>         } else if ((wpa_s->drv_flags & WPA_DRIVER_FLAGS_4WAY_HANDSHAKE) &&
>                    wpa_key_mgmt_wpa_ieee8021x(wpa_s->key_mgmt)) {
>                 /*
>                  * The driver will take care of RSN 4-way handshake, so
> we need
>                  * to allow EAPOL supplicant to complete its work without
>                  * waiting for WPA supplicant.
>                  */
>                 eapol_sm_notify_portValid(wpa_s->eapol, TRUE);
>         } else if (ft_completed) {
> 
> So for 4-way hs offload for wpa/wpa2-psk it already calls
> eapol_sm_notify_eap_success() and reading the documentation of that
> function:
> 
> /**
>  * eapol_sm_notify_eap_success - Notification of external EAP success
> trigger
>  * @sm: Pointer to EAPOL state machine allocated with eapol_sm_init()
>  * @success: %TRUE = set success, %FALSE = clear success
>  *
>  * Notify the EAPOL state machine that external event has forced EAP
> state to
>  * success (success = %TRUE). This can be cleared by setting success =
> %FALSE.
>  *
>  * This function is called to update EAP state when WPA-PSK key
> handshake has
>  * been completed successfully since WPA-PSK does not use EAP state machine.
>  */
> void eapol_sm_notify_eap_success(struct eapol_sm *sm, Boolean success)
> 
> So the code in wpa_supplicant_event_assoc() is either lying or the assoc
> event should come after driver has completed successfully. To me it
> seems worth it to have NL80211_CMD_EAP_COMPLETED event carrying the
> result or indeed give the CONNECTED event later in the driver.

Ehm. Looking at the code in wpa_supplicant_event_assoc() it would be
better to use NL80211_CMD_EAPOL_PORT_VALID event to cover both
WPA/WPA2-PSK and 8021X.

Regards,
Arend

> Regards,
> Arend
> 

Re: [PATCH] cfg80211: support 4-way handshake offloading for WPA/WPA2-PSK

[Jithu Jance]

On Thu, Feb 23, 2017 at 4:10 PM, Arend Van Spriel
<arend.vanspriel@broadcom.com> wrote:
>
> Ehm. Looking at the code in wpa_supplicant_event_assoc() it would be
> better to use NL80211_CMD_EAPOL_PORT_VALID event to cover both
> WPA/WPA2-PSK and 8021X.
Yes. IMHO, the assoc/reassoc ind should move the state to WPA_ASSOCIATED
and a separate event like NL80211_CMD_EAPOL_PORT_VALID/AUTHORIZED should
move the connection state to WPA_COMPLETED.


Thanks,
-- 

Jithu Jance

Re: [PATCH] cfg80211: support 4-way handshake offloading for WPA/WPA2-PSK

[Arend Van Spriel]

On 21-2-2017 12:46, Johannes Berg wrote:
> 
>>>> Would this work for you? We should have wpa_supplicant support
>>>> too, but need to ask Andrei to look at that.
>>
>> Forgot to reply to this. Yes, this will work. Can come up with
>> wpa_supp changes.
> 
> No, we have them. Just need to see where they are :)

Found anything ? :-p

Gr. AvS

Re: [PATCH] cfg80211: support 4-way handshake offloading for WPA/WPA2-PSK

[Johannes Berg]

On Thu, 2017-02-23 at 16:26 +0530, Jithu Jance wrote:
> On Thu, Feb 23, 2017 at 4:10 PM, Arend Van Spriel
> <arend.vanspriel@broadcom.com> wrote:
> > 
> > Ehm. Looking at the code in wpa_supplicant_event_assoc() it would
> > be
> > better to use NL80211_CMD_EAPOL_PORT_VALID event to cover both
> > WPA/WPA2-PSK and 8021X.
> 
> Yes. IMHO, the assoc/reassoc ind should move the state to
> WPA_ASSOCIATED and a separate event like
> NL80211_CMD_EAPOL_PORT_VALID/AUTHORIZED should move the connection
> state to WPA_COMPLETED.

That seems reasonable.

Avi just looked also at distinguishing if/when fresh 1X authentication
is required, particularly with roaming. For that, he's suggesting to
add a flag AUTHORIZED to the ROAMED event.

We could, possibly, have a PORT_AUTHORIZED event for that as well, but
it'd be more complicated, since then you'd have to wait for that and if
it doesn't come time out - or we'd need a "PORT_UNAUTHORIZED" or
"PLEASE_START_1X" instead? None of that really seems like such a great
idea.

Perhaps instead it'd make sense to instead include the new AUTHORIZED
flag in the CONNECT_RESULT as well, if authorized? I basically see 3
valid cases:

 * connection successful with authorized port
 * connection successful with need for 1X handshake (non-offloaded)
 * connection failed

Why should we have the case of
 * association successful but 4-way-HS failed

separately?

johannes

Re: [PATCH] cfg80211: support 4-way handshake offloading for WPA/WPA2-PSK

[Johannes Berg]

On Tue, 2017-02-28 at 12:06 +0100, Arend Van Spriel wrote:
> On 21-2-2017 12:46, Johannes Berg wrote:
> > 
> > > > > Would this work for you? We should have wpa_supplicant
> > > > > support
> > > > > too, but need to ask Andrei to look at that.
> > > 
> > > Forgot to reply to this. Yes, this will work. Can come up with
> > > wpa_supp changes.
> > 
> > No, we have them. Just need to see where they are :)
> 
> Found anything ? :-p

Sorry, got too busy with other stuff.

+Andrei, can you send out the patches from Avi in wpa_s for doing the
4-way-HS offload (as an RFC)?

Arend, wait a bit - he's travelling now I think.

Thanks,
johannes

Re: [PATCH] cfg80211: support 4-way handshake offloading for WPA/WPA2-PSK

[Arend Van Spriel]

On 3-3-2017 13:20, Johannes Berg wrote:
> On Thu, 2017-02-23 at 16:26 +0530, Jithu Jance wrote:
>> On Thu, Feb 23, 2017 at 4:10 PM, Arend Van Spriel
>> <arend.vanspriel@broadcom.com> wrote:
>>>
>>> Ehm. Looking at the code in wpa_supplicant_event_assoc() it would
>>> be
>>> better to use NL80211_CMD_EAPOL_PORT_VALID event to cover both
>>> WPA/WPA2-PSK and 8021X.
>>
>> Yes. IMHO, the assoc/reassoc ind should move the state to
>> WPA_ASSOCIATED and a separate event like
>> NL80211_CMD_EAPOL_PORT_VALID/AUTHORIZED should move the connection
>> state to WPA_COMPLETED.
> 
> That seems reasonable.
> 
> Avi just looked also at distinguishing if/when fresh 1X authentication
> is required, particularly with roaming. For that, he's suggesting to
> add a flag AUTHORIZED to the ROAMED event.
> 
> We could, possibly, have a PORT_AUTHORIZED event for that as well, but
> it'd be more complicated, since then you'd have to wait for that and if
> it doesn't come time out - or we'd need a "PORT_UNAUTHORIZED" or
> "PLEASE_START_1X" instead? None of that really seems like such a great
> idea.
> 
> Perhaps instead it'd make sense to instead include the new AUTHORIZED
> flag in the CONNECT_RESULT as well, if authorized? I basically see 3
> valid cases:
> 
>  * connection successful with authorized port
>  * connection successful with need for 1X handshake (non-offloaded)
>  * connection failed
> 
> Why should we have the case of
>  * association successful but 4-way-HS failed
> 
> separately?

I suppose there is no functional behavior that needs to done upon
association regardless 4-way HS result, ie. obtaining assoc ie info is
only useful if 4-way HS succeeds. I agree using the flag should be
sufficient.

Regards,
Arend

Re: [PATCH] cfg80211: support 4-way handshake offloading for WPA/WPA2-PSK

[Arend Van Spriel]

On 3-3-2017 13:21, Johannes Berg wrote:
> On Tue, 2017-02-28 at 12:06 +0100, Arend Van Spriel wrote:
>> On 21-2-2017 12:46, Johannes Berg wrote:
>>>
>>>>>> Would this work for you? We should have wpa_supplicant
>>>>>> support
>>>>>> too, but need to ask Andrei to look at that.
>>>>
>>>> Forgot to reply to this. Yes, this will work. Can come up with
>>>> wpa_supp changes.
>>>
>>> No, we have them. Just need to see where they are :)
>>
>> Found anything ? :-p
> 
> Sorry, got too busy with other stuff.
> 
> +Andrei, can you send out the patches from Avi in wpa_s for doing the
> 4-way-HS offload (as an RFC)?
> 
> Arend, wait a bit - he's travelling now I think.

Having a day off today so I can wait. Actually made the WPA-PSK change
myself in wpa_supplicant. It is working with brcmfmac. I did not look at
1X yet as I do not have a radius server up and running.

Regards,
Arend

Re: [PATCH] cfg80211: support 4-way handshake offloading for WPA/WPA2-PSK

[Arend Van Spriel]

On 3-3-2017 13:21, Johannes Berg wrote:
> On Tue, 2017-02-28 at 12:06 +0100, Arend Van Spriel wrote:
>> On 21-2-2017 12:46, Johannes Berg wrote:
>>>
>>>>>> Would this work for you? We should have wpa_supplicant
>>>>>> support
>>>>>> too, but need to ask Andrei to look at that.
>>>>
>>>> Forgot to reply to this. Yes, this will work. Can come up with
>>>> wpa_supp changes.
>>>
>>> No, we have them. Just need to see where they are :)
>>
>> Found anything ? :-p
> 
> Sorry, got too busy with other stuff.
> 
> +Andrei, can you send out the patches from Avi in wpa_s for doing the
> 4-way-HS offload (as an RFC)?
> 
> Arend, wait a bit - he's travelling now I think.

So the 4-way hs offload patches in wpa_s RFC do not make distinction
between 1X and WPA-PSK. Can we expect drivers to support one, but not
the other? In other words do we need the flexibility in nl80211?

Regards,
Arend

Re: [PATCH] cfg80211: support 4-way handshake offloading for WPA/WPA2-PSK

[Jithu Jance]

On Fri, Mar 3, 2017 at 5:50 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
>
> Perhaps instead it'd make sense to instead include the new AUTHORIZED
> flag in the CONNECT_RESULT as well, if authorized? I basically see 3
> valid cases:
>
>  * connection successful with authorized port
>  * connection successful with need for 1X handshake (non-offloaded)
>  * connection failed
>
> Why should we have the case of
>  * association successful but 4-way-HS failed
>
> separately?

Sorry for delayed reply. Makes sense. Authorized flag should do the
job. Thanks Johannes!


-- 

- Jithu Jance

Re: [PATCH] cfg80211: support 4-way handshake offloading for WPA/WPA2-PSK

[Johannes Berg]

> So the 4-way hs offload patches in wpa_s RFC do not make distinction
> between 1X and WPA-PSK. Can we expect drivers to support one, but not
> the other? In other words do we need the flexibility in nl80211?

You tell me :)

We're implementing both. Andrei also raised this question on the wpa_s
patches at my request, and nobody commented.

I can live with combining them, but nobody better complain about it
when they just implement one of them on their device ... :)

johannes

Re: [PATCH] cfg80211: support 4-way handshake offloading for WPA/WPA2-PSK

[Arend Van Spriel]

+ Jouni

On 14-3-2017 14:52, Johannes Berg wrote:
> 
>> So the 4-way hs offload patches in wpa_s RFC do not make distinction
>> between 1X and WPA-PSK. Can we expect drivers to support one, but not
>> the other? In other words do we need the flexibility in nl80211?
> 
> You tell me :)
> 
> We're implementing both. Andrei also raised this question on the wpa_s
> patches at my request, and nobody commented.

I see. For our firmware we can also implement both.

> I can live with combining them, but nobody better complain about it
> when they just implement one of them on their device ... :)

Indeed. These are two different use-cases so I can see merit in making
the distinction. However, for those implementing just one it would be
nice to have the distinction between 1X and WPA-PSK offload support in
wpa_supplicant as well.

Regards,
Arend