[PATCH 1/5] handshake: add HANDSHAKE_EVENT_EAP_COMPLETE

[PATCH 1/5] handshake: add HANDSHAKE_EVENT_EAP_COMPLETE

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 591 bytes --]

This is being added in order to support 8021x
offloading since there is no event available to
station which indicates that EAP has finished.
---
 src/handshake.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/handshake.h b/src/handshake.h
index b738efd9..3740d7f0 100644
--- a/src/handshake.h
+++ b/src/handshake.h
@@ -54,6 +54,7 @@ enum handshake_event {
 	HANDSHAKE_EVENT_FAILED,
 	HANDSHAKE_EVENT_REKEY_FAILED,
 	HANDSHAKE_EVENT_EAP_NOTIFY,
+	HANDSHAKE_EVENT_EAP_COMPLETE,
 };
 
 typedef void (*handshake_event_func_t)(struct handshake_state *hs,
-- 
2.26.2

[PATCH 2/5] eapol: notify with EAP complete handshake event

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 504 bytes --]

---
 src/eapol.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/eapol.c b/src/eapol.c
index 23aaf530..c4a73282 100644
--- a/src/eapol.c
+++ b/src/eapol.c
@@ -2162,7 +2162,8 @@ static void eapol_eap_complete_cb(enum eap_result result, void *user_data)
 		sm->eap = NULL;
 		handshake_failed(sm, MMPDU_REASON_CODE_IEEE8021X_FAILED);
 		return;
-	}
+	} else
+		handshake_event(sm->handshake, HANDSHAKE_EVENT_EAP_COMPLETE);
 
 	eap_reset(sm->eap);
 
-- 
2.26.2

[PATCH 3/5] netdev: implement netdev_handshake_state_set_pmk

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 4639 bytes --]

The 8021x offloading procedure still does EAP in userspace which
negotiates the PMK. The kernel then expects to obtain this PMK
from userspace by calling SET_PMK. This then allows the firmware
to begin the 4-way handshake.

netdev_handshake_set_pmk kicks off the final userspace
actions prior to the firmware starting the 4-way handshake:

 - SET_PMK using PMK negotiated with EAP
 - SET_STATION (concurrently with SET_PMK)
 - Emit SETTING_KEYS event
 - netdev_connect_ok

One thing to note is that the kernel provides no way of knowing if
the 4-way handshake completed. Assuming SET_PMK/SET_STATION come
back with no errors, IWD assumes the PMK was valid. If not, or
due to some other issue in the 4-way, the kernel will send a
disconnect.
---
 src/netdev.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/netdev.h |  2 ++
 2 files changed, 87 insertions(+)

diff --git a/src/netdev.c b/src/netdev.c
index d8703ed2..dc4aa5bc 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -82,6 +82,7 @@ struct netdev_handshake_state {
 	uint32_t group_new_key_cmd_id;
 	uint32_t group_management_new_key_cmd_id;
 	uint32_t set_station_cmd_id;
+	uint32_t set_pmk_cmd_id;
 	bool ptk_installed;
 	bool gtk_installed;
 	bool igtk_installed;
@@ -242,6 +243,11 @@ static void netdev_handshake_state_cancel_all(
 		l_genl_family_cancel(nl80211, nhs->set_station_cmd_id);
 		nhs->set_station_cmd_id = 0;
 	}
+
+	if (nhs->set_pmk_cmd_id) {
+		l_genl_family_cancel(nl80211, nhs->set_pmk_cmd_id);
+		nhs->set_pmk_cmd_id = 0;
+	}
 }
 
 static void netdev_handshake_state_free(struct handshake_state *hs)
@@ -1695,6 +1701,85 @@ invalid_key:
 	netdev_setting_keys_failed(nhs, err);
 }
 
+static void try_offload_handshake_complete(struct netdev_handshake_state *nhs,
+						int err)
+{
+	struct netdev *netdev = nhs->netdev;
+
+	if (err < 0) {
+		l_error("Error with SET_PMK/SET_STATION");
+		netdev_setting_keys_failed(nhs, err);
+		return;
+	}
+
+	if (nhs->set_pmk_cmd_id == 0 && nhs->set_station_cmd_id == 0) {
+		handshake_event(netdev->handshake,
+				HANDSHAKE_EVENT_SETTING_KEYS);
+		netdev_connect_ok(netdev);
+	}
+}
+
+static void netdev_set_pmk_cb(struct l_genl_msg *msg, void *user_data)
+{
+	struct netdev_handshake_state *nhs = user_data;
+	int err = l_genl_msg_get_error(msg);
+
+	nhs->set_pmk_cmd_id = 0;
+
+	try_offload_handshake_complete(nhs, err);
+}
+
+static void netdev_set_station_offload_cb(struct l_genl_msg *msg,
+						void *user_data)
+{
+	struct netdev_handshake_state *nhs = user_data;
+	int err = l_genl_msg_get_error(msg);
+
+	nhs->set_station_cmd_id = 0;
+
+	try_offload_handshake_complete(nhs, err);
+}
+
+void netdev_handshake_set_pmk(struct handshake_state *hs)
+{
+	struct l_genl_msg *msg;
+	struct netdev_handshake_state *nhs = l_container_of(hs,
+				struct netdev_handshake_state, super);
+	struct netdev *netdev = nhs->netdev;
+
+	/* Only relevent for 8021x offload */
+	if (nhs->type != CONNECTION_TYPE_8021X_OFFLOAD)
+		return;
+
+	msg = l_genl_msg_new(NL80211_CMD_SET_PMK);
+
+	l_genl_msg_append_attr(msg, NL80211_ATTR_IFINDEX, 4, &netdev->index);
+	l_genl_msg_append_attr(msg, NL80211_ATTR_MAC, 6, netdev->handshake->aa);
+	l_genl_msg_append_attr(msg, NL80211_ATTR_PMK,
+				netdev->handshake->pmk_len,
+				netdev->handshake->pmk);
+
+	nhs->set_pmk_cmd_id = l_genl_family_send(nl80211, msg,
+							netdev_set_pmk_cb,
+							nhs, NULL);
+	if (!nhs->set_pmk_cmd_id) {
+		l_error("Failed to set SET_PMK");
+		netdev_setting_keys_failed(nhs, -EIO);
+		return;
+	}
+
+	msg = nl80211_build_set_station_authorized(netdev->index,
+							netdev->handshake->aa);
+
+	nhs->set_station_cmd_id = l_genl_family_send(nl80211, msg,
+						netdev_set_station_offload_cb,
+						nhs, NULL);
+	if (!nhs->set_station_cmd_id) {
+		l_error("Failed to SET_STATION");
+		netdev_setting_keys_failed(nhs, -EIO);
+	}
+}
+
 void netdev_handshake_failed(struct handshake_state *hs, uint16_t reason_code)
 {
 	struct netdev_handshake_state *nhs =
diff --git a/src/netdev.h b/src/netdev.h
index c2c43290..2985e099 100644
--- a/src/netdev.h
+++ b/src/netdev.h
@@ -140,6 +140,8 @@ const char *netdev_get_path(struct netdev *netdev);
 struct handshake_state *netdev_handshake_state_new(struct netdev *netdev);
 struct handshake_state *netdev_get_handshake(struct netdev *netdev);
 
+void netdev_handshake_set_pmk(struct handshake_state *hs);
+
 int netdev_connect(struct netdev *netdev, struct scan_bss *bss,
 				struct handshake_state *hs,
 				const struct iovec *vendor_ies,
-- 
2.26.2

Re: [PATCH 3/5] netdev: implement netdev_handshake_state_set_pmk

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 2468 bytes --]

Hi James,

On 4/8/21 10:46 AM, James Prestwood wrote:
> The 8021x offloading procedure still does EAP in userspace which
> negotiates the PMK. The kernel then expects to obtain this PMK
> from userspace by calling SET_PMK. This then allows the firmware
> to begin the 4-way handshake.
> 
> netdev_handshake_set_pmk kicks off the final userspace
> actions prior to the firmware starting the 4-way handshake:
> 
>   - SET_PMK using PMK negotiated with EAP
>   - SET_STATION (concurrently with SET_PMK)
>   - Emit SETTING_KEYS event
>   - netdev_connect_ok
> 
> One thing to note is that the kernel provides no way of knowing if
> the 4-way handshake completed. Assuming SET_PMK/SET_STATION come
> back with no errors, IWD assumes the PMK was valid. If not, or
> due to some other issue in the 4-way, the kernel will send a
> disconnect.
> ---
>   src/netdev.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   src/netdev.h |  2 ++
>   2 files changed, 87 insertions(+)
> 

<snip>

> +void netdev_handshake_set_pmk(struct handshake_state *hs)
> +{
> +	struct l_genl_msg *msg;
> +	struct netdev_handshake_state *nhs = l_container_of(hs,
> +				struct netdev_handshake_state, super);
> +	struct netdev *netdev = nhs->netdev;
> +
> +	/* Only relevent for 8021x offload */
> +	if (nhs->type != CONNECTION_TYPE_8021X_OFFLOAD)
> +		return;
> +
> +	msg = l_genl_msg_new(NL80211_CMD_SET_PMK);
> +
> +	l_genl_msg_append_attr(msg, NL80211_ATTR_IFINDEX, 4, &netdev->index);
> +	l_genl_msg_append_attr(msg, NL80211_ATTR_MAC, 6, netdev->handshake->aa);
> +	l_genl_msg_append_attr(msg, NL80211_ATTR_PMK,
> +				netdev->handshake->pmk_len,
> +				netdev->handshake->pmk);
> +
> +	nhs->set_pmk_cmd_id = l_genl_family_send(nl80211, msg,
> +							netdev_set_pmk_cb,
> +							nhs, NULL);
> +	if (!nhs->set_pmk_cmd_id) {
> +		l_error("Failed to set SET_PMK");
> +		netdev_setting_keys_failed(nhs, -EIO);
> +		return;
> +	}
> +
> +	msg = nl80211_build_set_station_authorized(netdev->index,
> +							netdev->handshake->aa);

I seem to recall that brcmfmac reports a -ENOTSUP error for SET_STATION.  So is 
this even needed?

> +
> +	nhs->set_station_cmd_id = l_genl_family_send(nl80211, msg,
> +						netdev_set_station_offload_cb,
> +						nhs, NULL);
> +	if (!nhs->set_station_cmd_id) {
> +		l_error("Failed to SET_STATION");
> +		netdev_setting_keys_failed(nhs, -EIO);
> +	}
> +}
> +

Regards,
-Denis

Re: [PATCH 3/5] netdev: implement netdev_handshake_state_set_pmk

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 2855 bytes --]

On Thu, 2021-04-08 at 15:14 -0500, Denis Kenzior wrote:
> Hi James,
> 
> On 4/8/21 10:46 AM, James Prestwood wrote:
> > The 8021x offloading procedure still does EAP in userspace which
> > negotiates the PMK. The kernel then expects to obtain this PMK
> > from userspace by calling SET_PMK. This then allows the firmware
> > to begin the 4-way handshake.
> > 
> > netdev_handshake_set_pmk kicks off the final userspace
> > actions prior to the firmware starting the 4-way handshake:
> > 
> >   - SET_PMK using PMK negotiated with EAP
> >   - SET_STATION (concurrently with SET_PMK)
> >   - Emit SETTING_KEYS event
> >   - netdev_connect_ok
> > 
> > One thing to note is that the kernel provides no way of knowing if
> > the 4-way handshake completed. Assuming SET_PMK/SET_STATION come
> > back with no errors, IWD assumes the PMK was valid. If not, or
> > due to some other issue in the 4-way, the kernel will send a
> > disconnect.
> > ---
> >   src/netdev.c | 85
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >   src/netdev.h |  2 ++
> >   2 files changed, 87 insertions(+)
> > 
> 
> <snip>
> 
> > +void netdev_handshake_set_pmk(struct handshake_state *hs)
> > +{
> > +	struct l_genl_msg *msg;
> > +	struct netdev_handshake_state *nhs = l_container_of(hs,
> > +				struct netdev_handshake_state, super);
> > +	struct netdev *netdev = nhs->netdev;
> > +
> > +	/* Only relevent for 8021x offload */
> > +	if (nhs->type != CONNECTION_TYPE_8021X_OFFLOAD)
> > +		return;
> > +
> > +	msg = l_genl_msg_new(NL80211_CMD_SET_PMK);
> > +
> > +	l_genl_msg_append_attr(msg, NL80211_ATTR_IFINDEX, 4, &netdev-
> > >index);
> > +	l_genl_msg_append_attr(msg, NL80211_ATTR_MAC, 6, netdev-
> > >handshake->aa);
> > +	l_genl_msg_append_attr(msg, NL80211_ATTR_PMK,
> > +				netdev->handshake->pmk_len,
> > +				netdev->handshake->pmk);
> > +
> > +	nhs->set_pmk_cmd_id = l_genl_family_send(nl80211, msg,
> > +							netdev_set_pmk_
> > cb,
> > +							nhs, NULL);
> > +	if (!nhs->set_pmk_cmd_id) {
> > +		l_error("Failed to set SET_PMK");
> > +		netdev_setting_keys_failed(nhs, -EIO);
> > +		return;
> > +	}
> > +
> > +	msg = nl80211_build_set_station_authorized(netdev->index,
> > +							netdev-
> > >handshake->aa);
> 
> I seem to recall that brcmfmac reports a -ENOTSUP error for
> SET_STATION.  So is 
> this even needed?

Its accepting it just fine, but I tried removing it and it looks like
I'm still staying connected so apparently its not required :)

> 
> > +
> > +	nhs->set_station_cmd_id = l_genl_family_send(nl80211, msg,
> > +						netdev_set_station_offl
> > oad_cb,
> > +						nhs, NULL);
> > +	if (!nhs->set_station_cmd_id) {
> > +		l_error("Failed to SET_STATION");
> > +		netdev_setting_keys_failed(nhs, -EIO);
> > +	}
> > +}
> > +
> 
> Regards,
> -Denis

Re: [PATCH 3/5] netdev: implement netdev_handshake_state_set_pmk

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 379 bytes --]

Hi James,

>> I seem to recall that brcmfmac reports a -ENOTSUP error for
>> SET_STATION.  So is
>> this even needed?
> 
> Its accepting it just fine, but I tried removing it and it looks like
> I'm still staying connected so apparently its not required :)
> 

Okay, then I must be mis-remembering.  Maybe it was mwifiex and not brcmfmac 
actually.

Regards,
-Denis

[PATCH 4/5] netdev: add CONNECTION_TYPE_8021X_OFFLOAD

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 3296 bytes --]

This adds a new type for 8021x offload as well as support in
building CMD_CONNECT.

As described in the comment, 8021x offloading is not particularly
similar to PSK as far as the code flow in IWD is concerned. There
still needs to be an eapol_sm due to EAP being done in userspace.
This throws somewhat of a wrench into our 'is_offload' cases. And
as such this connection type is handled specially.
---
 src/netdev.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/src/netdev.c b/src/netdev.c
index dc4aa5bc..07d5e6aa 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -72,6 +72,7 @@ enum connection_type {
 	CONNECTION_TYPE_FULLMAC,
 	CONNECTION_TYPE_SAE_OFFLOAD,
 	CONNECTION_TYPE_PSK_OFFLOAD,
+	CONNECTION_TYPE_8021X_OFFLOAD,
 };
 
 static uint32_t unicast_watch;
@@ -204,6 +205,15 @@ static inline bool is_offload(struct handshake_state *hs)
 	switch (nhs->type) {
 	case CONNECTION_TYPE_SOFTMAC:
 	case CONNECTION_TYPE_FULLMAC:
+	/*
+	 * 8021x offload does not quite fit into the same category of PSK
+	 * offload. First the netdev_connect_event comes prior to EAP meaning
+	 * the handshake is not done at this point. In addition it still
+	 * requires EAP take place in userspace meaning IWD needs an eapol_sm.
+	 * Because of this, and our prior use of 'is_offload', it does not fit
+	 * into the same category and will need to be handled specially.
+	 */
+	case CONNECTION_TYPE_8021X_OFFLOAD:
 		return false;
 	case CONNECTION_TYPE_SAE_OFFLOAD:
 	case CONNECTION_TYPE_PSK_OFFLOAD:
@@ -2751,6 +2761,9 @@ static struct l_genl_msg *netdev_build_cmd_connect(struct netdev *netdev,
 	case CONNECTION_TYPE_PSK_OFFLOAD:
 		l_genl_msg_append_attr(msg, NL80211_ATTR_PMK, 32, hs->pmk);
 		break;
+	case CONNECTION_TYPE_8021X_OFFLOAD:
+		l_genl_msg_append_attr(msg, NL80211_ATTR_WANT_1X_4WAY_HS,
+					0, NULL);
 	}
 
 	if (prev_bssid)
@@ -3130,13 +3143,21 @@ static int netdev_handshake_state_setup_connection_type(
 		if (wiphy_has_ext_feature(wiphy,
 				NL80211_EXT_FEATURE_4WAY_HANDSHAKE_STA_PSK))
 			goto psk_offload;
-		/* fall through */
+
+		if (softmac)
+			goto softmac;
+
+		goto fullmac;
 	case IE_RSN_AKM_SUITE_8021X:
 	case IE_RSN_AKM_SUITE_FT_OVER_8021X:
 	case IE_RSN_AKM_SUITE_8021X_SHA256:
 	case IE_RSN_AKM_SUITE_8021X_SUITE_B_SHA256:
 	case IE_RSN_AKM_SUITE_8021X_SUITE_B_SHA384:
 	case IE_RSN_AKM_SUITE_FT_OVER_8021X_SHA384:
+		if (wiphy_has_ext_feature(wiphy,
+				NL80211_EXT_FEATURE_4WAY_HANDSHAKE_STA_1X))
+			goto offload_1x;
+
 		if (softmac)
 			goto softmac;
 
@@ -3181,6 +3202,9 @@ sae_offload:
 psk_offload:
 	nhs->type = CONNECTION_TYPE_PSK_OFFLOAD;
 	return 0;
+offload_1x:
+	nhs->type = CONNECTION_TYPE_8021X_OFFLOAD;
+	return 0;
 }
 
 static int netdev_connect_common(struct netdev *netdev,
@@ -3266,8 +3290,12 @@ build_cmd_connect:
 		if (!cmd_connect)
 			return -EINVAL;
 
-		if (!is_offload(hs) && (is_rsn || hs->settings_8021x))
+		if (!is_offload(hs) && (is_rsn || hs->settings_8021x)) {
 			sm = eapol_sm_new(hs);
+
+			if (nhs->type == CONNECTION_TYPE_8021X_OFFLOAD)
+				eapol_sm_set_require_handshake(sm, false);
+		}
 	}
 
 	return netdev_connect_common(netdev, cmd_connect, bss, hs, sm,
-- 
2.26.2

[PATCH 5/5] station: handle EAP_COMPLETE event

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 682 bytes --]

Notify netdev via netdev_handshake_set_pmk that EAP has finished.
---
 src/station.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/station.c b/src/station.c
index 064872c6..8dda113d 100644
--- a/src/station.c
+++ b/src/station.c
@@ -740,6 +740,10 @@ static void station_handshake_event(struct handshake_state *hs,
 		l_warn("Unable to securely rekey on this hw/kernel...");
 		station_reconnect(station);
 		break;
+	case HANDSHAKE_EVENT_EAP_COMPLETE:
+		l_debug("EAP complete");
+		netdev_handshake_set_pmk(hs);
+		break;
 	case HANDSHAKE_EVENT_COMPLETE:
 	case HANDSHAKE_EVENT_SETTING_KEYS_FAILED:
 	case HANDSHAKE_EVENT_EAP_NOTIFY:
-- 
2.26.2

Re: [PATCH 5/5] station: handle EAP_COMPLETE event

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1075 bytes --]

Hi James,

On 4/8/21 10:46 AM, James Prestwood wrote:
> Notify netdev via netdev_handshake_set_pmk that EAP has finished.
> ---
>   src/station.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/src/station.c b/src/station.c
> index 064872c6..8dda113d 100644
> --- a/src/station.c
> +++ b/src/station.c
> @@ -740,6 +740,10 @@ static void station_handshake_event(struct handshake_state *hs,
>   		l_warn("Unable to securely rekey on this hw/kernel...");
>   		station_reconnect(station);
>   		break;
> +	case HANDSHAKE_EVENT_EAP_COMPLETE:
> +		l_debug("EAP complete");
> +		netdev_handshake_set_pmk(hs);
> +		break;

Why not handle PMK setting just like ptk/gtk/igtk/rekey offload bits?  So maybe 
add __eapol_set_install_pmk_func() and inside the netdev implementation check 
whether the current connection type is OFFLOAD_8021X.  That would cut out 
station as a middle-man completely.

>   	case HANDSHAKE_EVENT_COMPLETE:
>   	case HANDSHAKE_EVENT_SETTING_KEYS_FAILED:
>   	case HANDSHAKE_EVENT_EAP_NOTIFY:
> 

Regards,
-Denis