[v2,3/4] ap: support PTK rekeys

Commit Message

James Prestwood Jan. 12, 2023, 7:32 p.m. UTC

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(+)

Comments

Denis Kenzior Jan. 13, 2023, 3:35 p.m. UTC | #1

Hi James,

On 1/12/23 13:32, James Prestwood wrote:
> 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(+)
> 

<snip>

> @@ -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);

Shouldn't you be cleaning up the timeout here?

> +}
> +
> +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;

Would checking sta->rekey_time == 0 also be worthwhile?  For stas that haven't 
authenticated yet?

> +
> +		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;

Ok, so you try to find the next soonest (absolute) rekey_time to schedule the 
next timeout.  Might be easier to just set next to ~0 and loop over the stations 
using l_time_before(sta->rekey_time, next), setting next as needed.

> +		}
> +
> +		ap_start_rekey(ap, sta);

And looks like this starts a rekey for any stations that we somehow missed?  How 
does this happen?

> +	}
> +
> +	/*
> +	 * 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;

Are you sure the rekey_timeout is destroyed here?

Might be easier to use l_timeout_modify instead of creating/destroying it all 
the time?

> +}
> +
Regards,
-Denis

Patch

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.