[PATCH 1/9] eapol: set secure on message 2/4 properly

[PATCH 1/9] eapol: set secure on message 2/4 properly

[James Prestwood]

The second handshake message was hard coded with the secure bit as
zero but for rekeys the secure bit should be set to 1. Fix this by
changing the 2/4 builder to take a boolean which will set the bit
properly.

It should be noted that hostapd doesn't check this bit so EAPoL
worked just fine, but IWD's checks are more strict.
---
 src/eapol.c | 12 +++++++-----
 src/eapol.h |  3 ++-
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/src/eapol.c b/src/eapol.c
index c7480c11..593daf41 100644
--- a/src/eapol.c
+++ b/src/eapol.c
@@ -766,11 +766,12 @@ struct eapol_key *eapol_create_ptk_2_of_4(
 				size_t extra_len,
 				const uint8_t *extra_data,
 				bool is_wpa,
-				size_t mic_len)
+				size_t mic_len,
+				bool secure)
 {
-	return eapol_create_common(protocol, version, false, key_replay_counter,
-					snonce, extra_len, extra_data, 1,
-					is_wpa, mic_len);
+	return eapol_create_common(protocol, version, secure,
+					key_replay_counter, snonce, extra_len,
+					extra_data, 1, is_wpa, mic_len);
 }
 
 struct eapol_key *eapol_create_ptk_4_of_4(
@@ -1326,7 +1327,8 @@ static void eapol_handle_ptk_1_of_4(struct eapol_sm *sm,
 					ek->key_descriptor_version,
 					L_BE64_TO_CPU(ek->key_replay_counter),
 					sm->handshake->snonce, ies_len, ies,
-					sm->handshake->wpa_ie, sm->mic_len);
+					sm->handshake->wpa_ie, sm->mic_len,
+					sm->rekey);
 
 	kck = handshake_state_get_kck(sm->handshake);
 
diff --git a/src/eapol.h b/src/eapol.h
index 8d8d5252..43dc224d 100644
--- a/src/eapol.h
+++ b/src/eapol.h
@@ -83,7 +83,8 @@ struct eapol_key *eapol_create_ptk_2_of_4(
 				size_t extra_len,
 				const uint8_t *extra_data,
 				bool is_wpa,
-				size_t mic_len);
+				size_t mic_len,
+				bool secure);
 
 struct eapol_key *eapol_create_ptk_4_of_4(
 				enum eapol_protocol_version protocol,
-- 
2.34.3

[PATCH 2/9] unit: update test-eapol with API change

[James Prestwood]

---
 unit/test-eapol.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/unit/test-eapol.c b/unit/test-eapol.c
index 227e485f..563541f5 100644
--- a/unit/test-eapol.c
+++ b/unit/test-eapol.c
@@ -1793,7 +1793,7 @@ static void eapol_4way_test(const void *data)
 				eapol_key_test_4.key_replay_counter,
 				snonce, eapol_key_test_4.key_data_len,
 				eapol_key_data_4 + EAPOL_FRAME_LEN(16),
-				false, 16);
+				false, 16, false);
 	assert(frame);
 	assert(eapol_calculate_mic(IE_RSN_AKM_SUITE_PSK, ptk, frame,
 					mic, 16));
@@ -1878,7 +1878,7 @@ static void eapol_wpa2_handshake_test(const void *data)
 				eapol_key_test_8.key_replay_counter,
 				snonce, eapol_key_test_8.key_data_len,
 				eapol_key_data_8 + EAPOL_FRAME_LEN(16),
-				false, 16);
+				false, 16, false);
 	assert(frame);
 	assert(eapol_calculate_mic(IE_RSN_AKM_SUITE_PSK, ptk, frame,
 					mic, 16));
@@ -2008,7 +2008,7 @@ static void eapol_wpa_handshake_test(const void *data)
 				eapol_key_test_14.key_replay_counter,
 				snonce, eapol_key_test_14.key_data_len,
 				eapol_key_data_14 + EAPOL_FRAME_LEN(16),
-				true, 16);
+				true, 16, false);
 	assert(frame);
 	assert(eapol_calculate_mic(IE_RSN_AKM_SUITE_PSK, ptk, frame,
 					mic, 16));
-- 
2.34.3

[PATCH 3/9] eapol: implement rekey support for authenticator

[James Prestwood]

The only changes required was to set the secure bit for message 1,
reset the frame retry counter, and don't explicitly set ptk_complete
to false

Initiating a rekey can now be done by simply calling eapol_start().
---
 src/eapol.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/eapol.c b/src/eapol.c
index 593daf41..d31116b6 100644
--- a/src/eapol.c
+++ b/src/eapol.c
@@ -1087,8 +1087,6 @@ static void eapol_send_ptk_1_of_4(struct eapol_sm *sm)
 
 	handshake_state_new_anonce(sm->handshake);
 
-	sm->handshake->ptk_complete = false;
-
 	sm->replay_counter++;
 
 	memset(ek, 0, EAPOL_FRAME_LEN(sm->mic_len));
@@ -1112,6 +1110,11 @@ static void eapol_send_ptk_1_of_4(struct eapol_sm *sm)
 
 	eapol_key_data_append(ek, sm->mic_len, HANDSHAKE_KDE_PMKID, pmkid, 16);
 
+	if (sm->handshake->ptk_complete) {
+		ek->secure = true;
+		sm->rekey = true;
+	}
+
 	ek->header.packet_len = L_CPU_TO_BE16(EAPOL_FRAME_LEN(sm->mic_len) +
 				EAPOL_KEY_DATA_LEN(ek, sm->mic_len) - 4);
 
@@ -2129,7 +2132,8 @@ static void eapol_handle_ptk_4_of_4(struct eapol_sm *sm,
 	 * This might be a retransmission, so accept but don't install
 	 * the keys again.
 	 */
-	if (!sm->handshake->ptk_complete)
+	if (!sm->handshake->ptk_complete ||
+				(sm->handshake->ptk_complete && sm->rekey))
 		handshake_state_install_ptk(sm->handshake);
 
 	sm->handshake->ptk_complete = true;
@@ -2483,6 +2487,8 @@ static void eapol_eap_complete_cb(enum eap_result result, void *user_data)
 
 		/* sm->mic_len will have been set in eapol_eap_results_cb */
 
+		sm->frame_retry = 0;
+
 		/* Kick off 4-Way Handshake */
 		eapol_ptk_1_of_4_retry(NULL, sm);
 	}
@@ -2874,6 +2880,8 @@ bool eapol_start(struct eapol_sm *sm)
 			if (L_WARN_ON(!sm->handshake->have_pmk))
 				return false;
 
+			sm->frame_retry = 0;
+
 			/* Kick off handshake */
 			eapol_ptk_1_of_4_retry(NULL, sm);
 		}
-- 
2.34.3

[PATCH 4/9] handshake: add event for rekey success

[James Prestwood]

Rekeys for station mode don't need to know when complete since
there is nothing to do once done. AP mode on the other hand needs
to know if the rekey was successful in order to reset/set the next
rekey timer.
---
 src/handshake.h | 1 +
 src/station.c   | 1 +
 2 files changed, 2 insertions(+)

diff --git a/src/handshake.h b/src/handshake.h
index f2321634..863ffac7 100644
--- a/src/handshake.h
+++ b/src/handshake.h
@@ -60,6 +60,7 @@ enum handshake_event {
 	HANDSHAKE_EVENT_EAP_NOTIFY,
 	HANDSHAKE_EVENT_TRANSITION_DISABLE,
 	HANDSHAKE_EVENT_P2P_IP_REQUEST,
+	HANDSHAKE_EVENT_REKEY_COMPLETE,
 };
 
 typedef void (*handshake_event_func_t)(struct handshake_state *hs,
diff --git a/src/station.c b/src/station.c
index bad067c8..ad5ad724 100644
--- a/src/station.c
+++ b/src/station.c
@@ -1015,6 +1015,7 @@ static void station_handshake_event(struct handshake_state *hs,
 	case HANDSHAKE_EVENT_SETTING_KEYS_FAILED:
 	case HANDSHAKE_EVENT_EAP_NOTIFY:
 	case HANDSHAKE_EVENT_P2P_IP_REQUEST:
+	case HANDSHAKE_EVENT_REKEY_COMPLETE:
 		/*
 		 * currently we don't care about any other events. The
 		 * netdev_connect_cb will notify us when the connection is
-- 
2.34.3

Re: [PATCH 4/9] handshake: add event for rekey success

[Denis Kenzior]

Hi James,

On 1/11/23 14:15, James Prestwood wrote:
> Rekeys for station mode don't need to know when complete since
> there is nothing to do once done. AP mode on the other hand needs
> to know if the rekey was successful in order to reset/set the next
> rekey timer.
> ---
>   src/handshake.h | 1 +
>   src/station.c   | 1 +
>   2 files changed, 2 insertions(+)
> 

Patches 4-7 applied, thanks.

Regards,
-Denis

[PATCH 5/9] netdev: unset ptk_installed flag for TK

[James Prestwood]

When the TK is installed the 'ptk_installed' flag was never set to
zero. For initial associations this was fine (already zero) but for
rekeys the flag needs to be unset so try_handshake_complete knows
if the key was installed. This is consistent with how gtk/igtk keys
work as well.
---
 src/netdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/netdev.c b/src/netdev.c
index cda8d183..fc8a2afa 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -2024,6 +2024,8 @@ static void netdev_set_tk(struct handshake_state *hs, uint8_t key_index,
 	const uint8_t *addr = netdev_choose_key_address(nhs);
 	int err;
 
+	nhs->ptk_installed = false;
+
 	/*
 	 * WPA1 does the group handshake after the 4-way finishes so we can't
 	 * rely on the gtk/igtk being set immediately after the ptk. Since
-- 
2.34.3

[PATCH 6/9] netdev: support HANDSHAKE_EVENT_REKEY_COMPLETE

[James Prestwood]

In try_handshake_complete() we return early if all the keys had
been installed before (initial associations). For rekeys we can
now emit the REKEY_COMPLETE event which lets AP mode reset the
rekey timer for that station.
---
 src/netdev.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/netdev.c b/src/netdev.c
index fc8a2afa..035dbd00 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -1469,8 +1469,13 @@ static void netdev_setting_keys_failed(struct netdev_handshake_state *nhs,
 
 static void try_handshake_complete(struct netdev_handshake_state *nhs)
 {
-	if (nhs->ptk_installed && nhs->gtk_installed && nhs->igtk_installed &&
-			!nhs->complete) {
+	if (nhs->ptk_installed && nhs->gtk_installed && nhs->igtk_installed) {
+		if (nhs->complete) {
+			handshake_event(&nhs->super,
+					HANDSHAKE_EVENT_REKEY_COMPLETE);
+			return;
+		}
+
 		nhs->complete = true;
 
 		if (handshake_event(&nhs->super, HANDSHAKE_EVENT_COMPLETE))
-- 
2.34.3

[PATCH 7/9] eapol: relax secure bit check on 2/4

[James Prestwood]

Old wpa_supplicant versions do not set the secure bit on 2/4 during
rekeys which causes IWD to reject the message and eventually time out.
Modern versions do set it correctly but even Android 13 (Pixel 5a)
still uses an ancient version of wpa_supplicant which does not set the
bit.

Relax this check and instead just print a warning but allow the message
to be processed.
---
 src/eapol.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/eapol.c b/src/eapol.c
index d31116b6..55c4b6d8 100644
--- a/src/eapol.c
+++ b/src/eapol.c
@@ -512,8 +512,7 @@ bool eapol_verify_ptk_2_of_4(const struct eapol_key *ek, bool ptk_complete)
 	if (!ek->key_mic)
 		return false;
 
-	if (ek->secure != ptk_complete)
-		return false;
+	L_WARN_ON(ek->secure != ptk_complete);
 
 	if (ek->encrypted_key_data)
 		return false;
-- 
2.34.3

[PATCH 8/9] ap: support PTK rekeys

[James Prestwood]

This adds support for rekeys to AP mode. A single timer is used and
reset to the next station needing a rekey. A default rekey timer of
600 seconds is used unless the profile sets a timeout.
---
 src/ap.c | 114 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 114 insertions(+)

diff --git a/src/ap.c b/src/ap.c
index 1d937103..ef819724 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -60,6 +60,8 @@
 #include "src/band.h"
 #include "src/common.h"
 
+#define AP_DEFAULT_REKEY_SECONDS 600
+
 struct ap_state {
 	struct netdev *netdev;
 	struct l_genl_family *nl80211;
@@ -106,6 +108,9 @@ struct ap_state {
 	struct l_dbus_message *scan_pending;
 	struct l_queue *networks;
 
+	struct l_timeout *rekey_timeout;
+	unsigned int rekey_time;
+
 	bool started : 1;
 	bool gtk_set : 1;
 	bool netconfig_set_addr4 : 1;
@@ -137,6 +142,7 @@ struct sta_state {
 	bool wsc_v2;
 	struct l_dhcp_lease *ip_alloc_lease;
 	bool ip_alloc_sent;
+	uint64_t rekey_time;
 
 	bool ht_support : 1;
 	bool ht_greenfield : 1;
@@ -345,6 +351,11 @@ static void ap_reset(struct ap_state *ap)
 		l_queue_destroy(ap->networks, l_free);
 		ap->networks = NULL;
 	}
+
+	if (ap->rekey_timeout) {
+		l_timeout_remove(ap->rekey_timeout);
+		ap->rekey_timeout = NULL;
+	}
 }
 
 static bool ap_event_done(struct ap_state *ap, bool prev_in_event)
@@ -377,6 +388,8 @@ static bool ap_event(struct ap_state *ap, enum ap_event_type event,
 	return ap_event_done(ap, prev);
 }
 
+static void ap_reset_rekey_timeout(struct ap_state *ap);
+
 static void ap_del_station(struct sta_state *sta, uint16_t reason,
 				bool disassociate)
 {
@@ -439,6 +452,89 @@ static void ap_del_station(struct sta_state *sta, uint16_t reason,
 
 		ap_event_done(ap, prev);
 	}
+
+	ap_reset_rekey_timeout(ap);
+}
+
+static void ap_start_rekey(struct ap_state *ap, struct sta_state *sta)
+{
+	l_debug("Rekey STA "MAC, MAC_STR(sta->addr));
+
+	eapol_start(sta->sm);
+}
+
+static void ap_rekey_timeout(struct l_timeout *timeout, void *user_data)
+{
+	struct ap_state *ap = user_data;
+
+	l_timeout_remove(timeout);
+
+	ap_reset_rekey_timeout(ap);
+}
+
+/*
+ * Used to initiate any rekeys which are due and reset the rekey timer to the
+ * next soonest station needing a rekey.
+ *
+ * TODO: Could adapt this to also take into account the next GTK rekey and
+ * service that as well. But GTK rekeys are not yet supported in AP mode.
+ */
+static void ap_reset_rekey_timeout(struct ap_state *ap)
+{
+	const struct l_queue_entry *e;
+	uint64_t now = l_time_now();
+	uint64_t next = 0;
+
+	if (!ap->rekey_time)
+		return;
+
+	/* Find the station(s) that need a rekey and start it */
+	for (e = l_queue_get_entries(ap->sta_states); e; e = e->next) {
+		struct sta_state *sta = e->data;
+
+		if (!sta->associated || !sta->rsna)
+			continue;
+
+		if (l_time_before(now, sta->rekey_time)) {
+			uint64_t diff = l_time_diff(now, sta->rekey_time);
+
+			/* Finding the next rekey time */
+			if (next < diff)
+				next = diff;
+
+			continue;
+		}
+
+		ap_start_rekey(ap, sta);
+	}
+
+	/*
+	 * Set the next rekey to the station needing it the soonest, or NULL
+	 * if a single station and wait until the rekey is complete to reset
+	 * the timer.
+	 */
+	if (next)
+		ap->rekey_timeout = l_timeout_create(l_time_to_secs(next),
+						ap_rekey_timeout, ap, NULL);
+	else
+		ap->rekey_timeout = NULL;
+}
+
+static void ap_set_sta_rekey_timer(struct ap_state *ap, struct sta_state *sta)
+{
+	if (!ap->rekey_time)
+		return;
+
+	sta->rekey_time = l_time_now() + ap->rekey_time - 1;
+
+	/*
+	 * First/only station authenticated, set rekey timer. Any more stations
+	 * will just set their rekey time and be serviced by the single callback
+	 */
+	if (!ap->rekey_timeout)
+		ap->rekey_timeout = l_timeout_create(
+						l_time_to_secs(ap->rekey_time),
+						ap_rekey_timeout, ap, NULL);
 }
 
 static bool ap_sta_match_addr(const void *a, const void *b)
@@ -479,6 +575,8 @@ static void ap_new_rsna(struct sta_state *sta)
 
 	sta->rsna = true;
 
+	ap_set_sta_rekey_timer(ap, sta);
+
 	event_data.mac = sta->addr;
 	event_data.assoc_ies = sta->assoc_ies;
 	event_data.assoc_ies_len = sta->assoc_ies_len;
@@ -1372,6 +1470,9 @@ static void ap_handshake_event(struct handshake_state *hs,
 		sta->hs->go_ip_addr = IP4_FROM_STR(own_addr_str);
 		break;
 	}
+	case HANDSHAKE_EVENT_REKEY_COMPLETE:
+		ap_set_sta_rekey_timer(ap, sta);
+		return;
 	default:
 		break;
 	}
@@ -3628,6 +3729,19 @@ static int ap_load_config(struct ap_state *ap, const struct l_settings *config,
 		l_strfreev(strvval);
 	}
 
+	if (l_settings_has_key(config, "General", "RekeyTimeout")) {
+		unsigned int uintval;
+
+		if (!l_settings_get_uint(config, "General",
+						"RekeyTimeout", &uintval)) {
+			l_error("AP [General].RekeyTimeout is not valid");
+			return -EINVAL;
+		}
+
+		ap->rekey_time = uintval * L_USEC_PER_SEC;
+	} else
+		ap->rekey_time = AP_DEFAULT_REKEY_SECONDS * L_USEC_PER_SEC;
+
 	/*
 	 * Since 5GHz won't ever support only CCK rates we can ignore this
 	 * setting on that band.
-- 
2.34.3

[PATCH 9/9] doc: Document RekeyTimeout for AP profiles

[James Prestwood]

---
 src/iwd.ap.rst | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/iwd.ap.rst b/src/iwd.ap.rst
index 823aba99..ce402f91 100644
--- a/src/iwd.ap.rst
+++ b/src/iwd.ap.rst
@@ -67,6 +67,13 @@ The group ``[General]`` contains general AP configuration.
        ensure the country is set, and that the desired frequency/channel is
        unrestricted.
 
+   * - RekeyTimeout
+     - Timeout for PTK rekeys (seconds)
+
+       The time interval at which the AP starts a rekey for a given station. If
+       not provided a default value of 600 seconds is used. A value of 0 will
+       disable PTK rekeys completely.
+
 Network Authentication Settings
 -------------------------------
 
-- 
2.34.3

Re: [PATCH 1/9] eapol: set secure on message 2/4 properly

[Denis Kenzior]

Hi James,

On 1/11/23 14:15, James Prestwood wrote:
> The second handshake message was hard coded with the secure bit as
> zero but for rekeys the secure bit should be set to 1. Fix this by
> changing the 2/4 builder to take a boolean which will set the bit
> properly.
> 
> It should be noted that hostapd doesn't check this bit so EAPoL
> worked just fine, but IWD's checks are more strict.
> ---
>   src/eapol.c | 12 +++++++-----
>   src/eapol.h |  3 ++-
>   2 files changed, 9 insertions(+), 6 deletions(-)
> 

Patch 1 & 2 applied, thanks.

Regards,
-Denis