[PATCH 1/4] dpp: factor out key derivation and starting PKEX into functions

[PATCH 1/4] dpp: factor out key derivation and starting PKEX into functions

[James Prestwood]

This will make things a bit easier in future patches, and reduces
some of the length/complexity of these functions.
---
 src/dpp.c | 92 ++++++++++++++++++++++++++++++-------------------------
 1 file changed, 51 insertions(+), 41 deletions(-)

diff --git a/src/dpp.c b/src/dpp.c
index d89c3056..c1047ca3 100644
--- a/src/dpp.c
+++ b/src/dpp.c
@@ -4280,6 +4280,17 @@ static uint32_t *dpp_default_freqs(struct dpp_sm *dpp, size_t *out_len)
 	return freqs_out;
 }
 
+static void __dpp_pkex_start_enrollee(struct dpp_sm *dpp)
+{
+	dpp->current_freq = dpp->freqs[0];
+
+	dpp_reset_protocol_timer(dpp, DPP_PKEX_PROTO_TIMEOUT);
+
+	l_debug("PKEX start enrollee (id=%s)", dpp->pkex_id ?: "unset");
+
+	dpp_start_offchannel(dpp, dpp->current_freq);
+}
+
 static bool dpp_pkex_scan_notify(int err, struct l_queue *bss_list,
 					const struct scan_freq_set *freqs,
 					void *user_data)
@@ -4314,13 +4325,7 @@ static bool dpp_pkex_scan_notify(int err, struct l_queue *bss_list,
 	dpp->freqs = scan_freq_set_to_fixed_array(freq_set, &dpp->freqs_len);
 
 start:
-	dpp->current_freq = dpp->freqs[0];
-
-	dpp_reset_protocol_timer(dpp, DPP_PKEX_PROTO_TIMEOUT);
-
-	l_debug("PKEX start enrollee (id=%s)", dpp->pkex_id ?: "unset");
-
-	dpp_start_offchannel(dpp, dpp->current_freq);
+	__dpp_pkex_start_enrollee(dpp);
 
 	return false;
 
@@ -4338,40 +4343,6 @@ static void dpp_pkex_scan_destroy(void *user_data)
 
 static bool dpp_start_pkex_enrollee(struct dpp_sm *dpp)
 {
-	_auto_(l_ecc_point_free) struct l_ecc_point *qi = NULL;
-
-	memcpy(dpp->peer_addr, broadcast, 6);
-
-	/*
-	 * In theory a driver could support a lesser duration than 200ms. This
-	 * complicates things since we would need to tack on additional
-	 * offchannel requests to meet the 200ms requirement. This could be done
-	 * but for now use max_roc or 200ms, whichever is less.
-	 */
-	dpp->dwell = (dpp->max_roc < 200) ? dpp->max_roc : 200;
-	/* "DPP R2 devices are expected to use PKEXv1 by default" */
-	dpp->pkex_version = 1;
-
-	if (!l_ecdh_generate_key_pair(dpp->curve, &dpp->pkex_private,
-					&dpp->pkex_public))
-		goto failed;
-
-	/*
-	 * "If Qi is the point-at-infinity, the code shall be deleted and the
-	 * user should be notified to provision a new code"
-	 */
-	qi = dpp_derive_qi(dpp->curve, dpp->pkex_key, dpp->pkex_id,
-				netdev_get_address(dpp->netdev));
-	if (!qi || l_ecc_point_is_infinity(qi)) {
-		l_debug("Cannot derive Qi, provision a new code");
-		goto failed;
-	}
-
-	dpp->pkex_m = l_ecc_point_new(dpp->curve);
-
-	if (!l_ecc_point_add(dpp->pkex_m, dpp->pkex_public, qi))
-		goto failed;
-
 	dpp_property_changed_notify(dpp);
 
 	/*
@@ -4438,6 +4409,40 @@ static bool dpp_parse_pkex_args(struct l_dbus_message *message,
 	return true;
 }
 
+static bool dpp_pkex_derive_keys(struct dpp_sm *dpp)
+{
+	_auto_(l_ecc_point_free) struct l_ecc_point *qi = NULL;
+
+	/*
+	 * In theory a driver could support a lesser duration than 200ms. This
+	 * complicates things since we would need to tack on additional
+	 * offchannel requests to meet the 200ms requirement. This could be done
+	 * but for now use max_roc or 200ms, whichever is less.
+	 */
+	dpp->dwell = (dpp->max_roc < 200) ? dpp->max_roc : 200;
+	/* "DPP R2 devices are expected to use PKEXv1 by default" */
+	dpp->pkex_version = 1;
+
+	if (!l_ecdh_generate_key_pair(dpp->curve, &dpp->pkex_private,
+					&dpp->pkex_public))
+		return false;
+
+	/*
+	 * "If Qi is the point-at-infinity, the code shall be deleted and the
+	 * user should be notified to provision a new code"
+	 */
+	qi = dpp_derive_qi(dpp->curve, dpp->pkex_key, dpp->pkex_id,
+				netdev_get_address(dpp->netdev));
+	if (!qi || l_ecc_point_is_infinity(qi)) {
+		l_debug("Cannot derive Qi, provision a new code");
+		return false;
+	}
+
+	dpp->pkex_m = l_ecc_point_new(dpp->curve);
+
+	return l_ecc_point_add(dpp->pkex_m, dpp->pkex_public, qi);
+}
+
 static struct l_dbus_message *dpp_dbus_pkex_start_enrollee(struct l_dbus *dbus,
 						struct l_dbus_message *message,
 						void *user_data)
@@ -4472,6 +4477,11 @@ static struct l_dbus_message *dpp_dbus_pkex_start_enrollee(struct l_dbus *dbus,
 		return dbus_error_from_errno(ret, message);
 	}
 
+	if (!dpp_pkex_derive_keys(dpp)) {
+		dpp_reset(dpp);
+		return dbus_error_failed(message);
+	}
+
 	if (!wait_for_disconnect && !dpp_start_pkex_enrollee(dpp))
 		goto invalid_args;
 
-- 
2.34.1

[PATCH 2/4] dpp: add Address/Frequency as parameters to PKEX enrollees

[James Prestwood]

The DPP spec allows for obtaining frequency and MAC addresses up
to the implementation. IWD already takes advantage of this by
first scanning for nearby APs and using only those frequencies.
For further optimization an enrollee may be able to determine the
configurators frequency and MAC ahead of time which would make
finding the configurator much faster.
---
 src/dpp.c | 43 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 40 insertions(+), 3 deletions(-)

diff --git a/src/dpp.c b/src/dpp.c
index c1047ca3..39b66154 100644
--- a/src/dpp.c
+++ b/src/dpp.c
@@ -4345,6 +4345,12 @@ static bool dpp_start_pkex_enrollee(struct dpp_sm *dpp)
 {
 	dpp_property_changed_notify(dpp);
 
+	/* Already have a set (or single) frequency */
+	if (dpp->freqs) {
+		__dpp_pkex_start_enrollee(dpp);
+		return true;
+	}
+
 	/*
 	 * The 'dpp_default_freqs' function returns the default frequencies
 	 * outlined in section 5.6.1. For 2.4/5GHz this is only 3 frequencies
@@ -4374,13 +4380,17 @@ failed:
 
 static bool dpp_parse_pkex_args(struct l_dbus_message *message,
 					const char **key_out,
-					const char **id_out)
+					const char **id_out,
+					const char **mac_out,
+					uint32_t *freq_out)
 {
 	struct l_dbus_message_iter iter;
 	struct l_dbus_message_iter variant;
 	const char *dict_key;
 	const char *key = NULL;
 	const char *id = NULL;
+	const char *mac = NULL;
+	uint32_t freq = 0;
 
 	if (!l_dbus_message_get_arguments(message, "a{sv}", &iter))
 		return false;
@@ -4394,6 +4404,14 @@ static bool dpp_parse_pkex_args(struct l_dbus_message *message,
 			if (!l_dbus_message_iter_get_variant(&variant, "s",
 								&id))
 				return false;
+		} else if (!strcmp(dict_key, "Address")) {
+			if (!l_dbus_message_iter_get_variant(&variant, "s",
+								&mac))
+				return false;
+		} else if (!strcmp(dict_key, "Frequency")) {
+			if (!l_dbus_message_iter_get_variant(&variant, "u",
+								&freq))
+				return false;
 		}
 	}
 
@@ -4406,6 +4424,12 @@ static bool dpp_parse_pkex_args(struct l_dbus_message *message,
 	*key_out = key;
 	*id_out = id;
 
+	if (mac_out)
+		*mac_out = mac;
+
+	if (freq_out)
+		*freq_out = freq;
+
 	return true;
 }
 
@@ -4450,6 +4474,8 @@ static struct l_dbus_message *dpp_dbus_pkex_start_enrollee(struct l_dbus *dbus,
 	struct dpp_sm *dpp = user_data;
 	const char *key;
 	const char *id;
+	const char *mac_str;
+	uint32_t freq;
 	bool wait_for_disconnect;
 	int ret;
 
@@ -4459,8 +4485,19 @@ static struct l_dbus_message *dpp_dbus_pkex_start_enrollee(struct l_dbus *dbus,
 				dpp->interface != DPP_INTERFACE_UNBOUND)
 		return dbus_error_busy(message);
 
-	if (!dpp_parse_pkex_args(message, &key, &id))
+	if (!dpp_parse_pkex_args(message, &key, &id, &mac_str, &freq))
+		goto invalid_args;
+
+	if (mac_str && !util_string_to_address(mac_str, dpp->peer_addr))
 		goto invalid_args;
+	else if (!mac_str)
+		memcpy(dpp->peer_addr, broadcast, 6);
+
+	if (freq) {
+		dpp->freqs = l_new(uint32_t, 1);
+		dpp->freqs[0] = freq;
+		dpp->freqs_len = 1;
+	}
 
 	dpp->pkex_key = l_strdup(key);
 
@@ -4595,7 +4632,7 @@ static struct l_dbus_message *dpp_dbus_pkex_configure_enrollee(
 
 	l_debug("");
 
-	if (!dpp_parse_pkex_args(message, &key, &id))
+	if (!dpp_parse_pkex_args(message, &key, &id, NULL, NULL))
 		return dbus_error_invalid_args(message);
 
 	return dpp_start_pkex_configurator(dpp, key, id, NULL, message);
-- 
2.34.1

[PATCH 3/4] dpp: allow PKEX configurators to run without multicast RX support

[James Prestwood]

Since IWD enrollees can send unicast frames, a PKEX configurator could
still run without multicast support. Using this combination basically
allows any driver to utilize DPP/PKEX assuming the MAC address can
be communicated using some out of band mechanism.
---
 src/dpp.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/src/dpp.c b/src/dpp.c
index 39b66154..dad74efe 100644
--- a/src/dpp.c
+++ b/src/dpp.c
@@ -4574,11 +4574,9 @@ static struct l_dbus_message *dpp_start_pkex_configurator(struct dpp_sm *dpp,
 				dpp->interface != DPP_INTERFACE_UNBOUND)
 		return dbus_error_busy(message);
 
-	if (!dpp->mcast_support) {
-		l_debug("Multicast frame registration not supported, cannot "
-			"start a configurator");
-		return dbus_error_not_supported(message);
-	}
+	if (!dpp->mcast_support)
+		l_debug("Multicast frame registration not supported, only "
+			"enrollees sending uncast will be supported");
 
 	if (!network || !bss)
 		return dbus_error_not_connected(message);
-- 
2.34.1

[PATCH 4/4] dpp: scale PKEX timeout by the number of frequencies used

[James Prestwood]

If the number of frequencies used is very small reduce the timeout
to avoid waiting for extended periods of time.
---
 src/dpp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/dpp.c b/src/dpp.c
index dad74efe..5d56456d 100644
--- a/src/dpp.c
+++ b/src/dpp.c
@@ -59,6 +59,7 @@
 #define DPP_FRAME_RETRY_TIMEOUT 1
 #define DPP_AUTH_PROTO_TIMEOUT 10
 #define DPP_PKEX_PROTO_TIMEOUT 120
+#define DPP_PKEX_PROTO_PER_FREQ_TIMEOUT 10
 
 static uint32_t netdev_watch;
 static struct l_genl_family *nl80211;
@@ -4284,7 +4285,8 @@ static void __dpp_pkex_start_enrollee(struct dpp_sm *dpp)
 {
 	dpp->current_freq = dpp->freqs[0];
 
-	dpp_reset_protocol_timer(dpp, DPP_PKEX_PROTO_TIMEOUT);
+	dpp_reset_protocol_timer(dpp,
+			dpp->freqs_len * DPP_PKEX_PROTO_PER_FREQ_TIMEOUT);
 
 	l_debug("PKEX start enrollee (id=%s)", dpp->pkex_id ?: "unset");
 
-- 
2.34.1

Re: [PATCH 1/4] dpp: factor out key derivation and starting PKEX into functions

[Denis Kenzior]

Hi James,

On 8/27/24 9:21 AM, James Prestwood wrote:
> This will make things a bit easier in future patches, and reduces
> some of the length/complexity of these functions.
> ---
>   src/dpp.c | 92 ++++++++++++++++++++++++++++++-------------------------
>   1 file changed, 51 insertions(+), 41 deletions(-)
> 

All applied, thanks.

Regards,
-Denis