[PATCH 0/2] Shared code scan discovery (improvement)

[PATCH 0/2] Shared code scan discovery (improvement)

[James Prestwood]

We had talked about this idea but wanted to leave it out until the
initial PKEX code was in. If we want, we can make this the default
behavior because without it PKEX between two IWD clients is unlikely
to ever work since there is no way to specify the frequency. You'd
just have to get lucky and have the AP be on one of the 3 channels.

I don't really see a downside to making it the default and removing
the option entirely, but I left it in because thats what I had in
my tree already. IMO anyone using this feature would want to set
this option.

James Prestwood (2):
  dpp: scan to pick up extra frequencies when enrolling
  doc: document [DeviceProvisioning].EnrolleeScanDiscovery

 src/dpp.c          | 93 ++++++++++++++++++++++++++++++++++++++++++++++
 src/iwd.config.rst | 19 ++++++++++
 2 files changed, 112 insertions(+)

-- 
2.25.1

[PATCH 1/2] dpp: scan to pick up extra frequencies when enrolling

[James Prestwood]

The DPP-PKEX spec provides a very limited list of frequencies used
to discover configurators, only 3 on 2.4 and 5GHz bands. Since
configurators (at least in IWD's implementation) are only allowed
on the current operating frequency its very unlikely an enrollee
will find a configurator on these frequencies out of the entire
spectrum.

The spec does mention that the 3 default frequencies should be used
"In lieu of specific channel information obtained in a manner outside
the scope of this specification, ...". This allows the implementation
some flexibility in using a broader range of frequencies.

To increase the chances of finding a configurator shared code
enrollees will first issue a scan to determine what access points are
around, then iterate these frequencies. This is especially helpful
when the configurators are IWD-based since we know that they'll be
on the same channels as the APs in the area.
---
 src/dpp.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)

diff --git a/src/dpp.c b/src/dpp.c
index c54bd484..9be5241d 100644
--- a/src/dpp.c
+++ b/src/dpp.c
@@ -182,6 +182,7 @@ struct dpp_sm {
 	size_t z_len;
 	uint8_t u[L_ECC_SCALAR_MAX_BYTES];
 	size_t u_len;
+	uint32_t pkex_scan_id;
 
 	bool mcast_support : 1;
 	bool roc_started : 1;
@@ -507,6 +508,11 @@ static void dpp_reset(struct dpp_sm *dpp)
 		dpp->retry_timeout = NULL;
 	}
 
+	if (dpp->pkex_scan_id) {
+		scan_cancel(dpp->wdev_id, dpp->pkex_scan_id);
+		dpp->pkex_scan_id = 0;
+	}
+
 	dpp->state = DPP_STATE_NOTHING;
 	dpp->new_freq = 0;
 	dpp->frame_retry = 0;
@@ -3956,6 +3962,14 @@ static struct l_dbus_message *dpp_dbus_stop(struct l_dbus *dbus,
 	return l_dbus_message_new_method_return(message);
 }
 
+static void dpp_pkex_scan_trigger(int err, void *user_data)
+{
+	struct dpp_sm *dpp = user_data;
+
+	if (err < 0)
+		dpp_reset(dpp);
+}
+
 /*
  * Section 5.6.1
  * In lieu of specific channel information obtained in a manner outside
@@ -3994,10 +4008,64 @@ static uint32_t *dpp_default_freqs(struct dpp_sm *dpp, size_t *out_len)
 	return freqs_out;
 }
 
+static bool dpp_pkex_scan_notify(int err, struct l_queue *bss_list,
+					const struct scan_freq_set *freqs,
+					void *user_data)
+{
+	struct dpp_sm *dpp = user_data;
+	const struct l_queue_entry *e;
+	_auto_(scan_freq_set_free) struct scan_freq_set *freq_set = NULL;
+
+	if (err < 0) {
+		dpp_reset(dpp);
+		return false;
+	}
+
+	freq_set = scan_freq_set_new();
+
+	if (!bss_list || l_queue_isempty(bss_list)) {
+		dpp->freqs = dpp_default_freqs(dpp, &dpp->freqs_len);
+
+		l_debug("No BSS's seen, using default frequency list");
+		goto start;
+	}
+
+	for (e = l_queue_get_entries(bss_list); e; e = e->next) {
+		const struct scan_bss *bss = e->data;
+
+		scan_freq_set_add(freq_set, bss->frequency);
+	}
+
+	l_debug("Found %u frequencies to search for configurator",
+			l_queue_length(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);
+
+	return false;
+}
+
+static void dpp_pkex_scan_destroy(void *user_data)
+{
+	struct dpp_sm *dpp = user_data;
+
+	dpp->pkex_scan_id = 0;
+}
+
 static bool dpp_start_pkex_enrollee(struct dpp_sm *dpp, const char *key,
 				const char *identifier)
 {
 	struct station *station = station_find(netdev_get_ifindex(dpp->netdev));
+	bool scan_discovery;
+	const struct l_settings *config = iwd_get_config();
 	_auto_(l_ecc_point_free) struct l_ecc_point *qi = NULL;
 
 	if (station && station_get_connected_network(station)) {
@@ -4045,6 +4113,31 @@ static bool dpp_start_pkex_enrollee(struct dpp_sm *dpp, const char *key,
 
 	dpp_property_changed_notify(dpp);
 
+	/*
+	 * The 'dpp_default_freqs' function returns the default frequencies
+	 * outlined in section 5.6.1. For 2.4/5GHz this is only 3 frequencies
+	 * which is unlikely to result in discovery of a configurator. The spec
+	 * does allow frequencies to be "obtained in a manner outside the scope
+	 * of this specification" which is what is being done here.
+	 *
+	 * This is mainly geared towards IWD-based configurators; banking on the
+	 * fact that they are currently connected to nearby APs. Scanning lets
+	 * us see nearby BSS's which should be the same frequencies as our
+	 * target configurator.
+	 */
+	if (l_settings_get_bool(config, "DeviceProvisioning",
+				"EnrolleeScanDiscovery", &scan_discovery) &&
+				scan_discovery) {
+		l_debug("Performing scan for frequencies to start PKEX");
+		dpp->pkex_scan_id = scan_active(dpp->wdev_id, NULL, 0,
+				dpp_pkex_scan_trigger, dpp_pkex_scan_notify,
+				dpp, dpp_pkex_scan_destroy);
+		if (!dpp->pkex_scan_id)
+			goto failed;
+
+		return true;
+	}
+
 	dpp->freqs = dpp_default_freqs(dpp, &dpp->freqs_len);
 	if (!dpp->freqs)
 		goto failed;
-- 
2.25.1

[PATCH 2/2] doc: document [DeviceProvisioning].EnrolleeScanDiscovery

[James Prestwood]

---
 src/iwd.config.rst | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/src/iwd.config.rst b/src/iwd.config.rst
index d9c94e01..794446b6 100644
--- a/src/iwd.config.rst
+++ b/src/iwd.config.rst
@@ -413,6 +413,25 @@ are buggy or just don't behave similar enough to the majority of other drivers.
 
        If a driver in user matches one in this list power save will be disabled.
 
+DeviceProvisioning
+------------------
+
+The group ``[DeviceProvisioning]`` contains settings used for the DPP (Device
+Provisioning Protocol).
+
+.. list-table::
+   :header-rows: 0
+   :stub-columns: 0
+   :widths: 20 80
+   :align: left
+
+   * - EnrolleeScanDiscovery
+     - Values: true, **false**
+
+       When enabled a shared code enrollee will first scan and use the results
+       to determine which frequencies to iterate to discover a configurator.
+       This greatly increases the chances of a configurator being found.
+
 SEE ALSO
 ========
 
-- 
2.25.1

Re: [PATCH 1/2] dpp: scan to pick up extra frequencies when enrolling

[Denis Kenzior]

Hi James,

On 11/9/23 13:57, James Prestwood wrote:
> The DPP-PKEX spec provides a very limited list of frequencies used
> to discover configurators, only 3 on 2.4 and 5GHz bands. Since
> configurators (at least in IWD's implementation) are only allowed
> on the current operating frequency its very unlikely an enrollee
> will find a configurator on these frequencies out of the entire
> spectrum.
> 
> The spec does mention that the 3 default frequencies should be used
> "In lieu of specific channel information obtained in a manner outside
> the scope of this specification, ...". This allows the implementation
> some flexibility in using a broader range of frequencies.
> 
> To increase the chances of finding a configurator shared code
> enrollees will first issue a scan to determine what access points are
> around, then iterate these frequencies. This is especially helpful
> when the configurators are IWD-based since we know that they'll be
> on the same channels as the APs in the area.
> ---
>   src/dpp.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 93 insertions(+)
> 

Ok, sounds good to me.

> @@ -3994,10 +4008,64 @@ static uint32_t *dpp_default_freqs(struct dpp_sm *dpp, size_t *out_len)
>   	return freqs_out;
>   }
>   
> +static bool dpp_pkex_scan_notify(int err, struct l_queue *bss_list,
> +					const struct scan_freq_set *freqs,
> +					void *user_data)
> +{
> +	struct dpp_sm *dpp = user_data;
> +	const struct l_queue_entry *e;
> +	_auto_(scan_freq_set_free) struct scan_freq_set *freq_set = NULL;
> +
> +	if (err < 0) {
> +		dpp_reset(dpp);
> +		return false;
> +	}
> +
> +	freq_set = scan_freq_set_new();
> +
> +	if (!bss_list || l_queue_isempty(bss_list)) {
> +		dpp->freqs = dpp_default_freqs(dpp, &dpp->freqs_len);

This can return NULL.

> +
> +		l_debug("No BSS's seen, using default frequency list");
> +		goto start;
> +	}
> +
> +	for (e = l_queue_get_entries(bss_list); e; e = e->next) {
> +		const struct scan_bss *bss = e->data;
> +
> +		scan_freq_set_add(freq_set, bss->frequency);
> +	}
> +
> +	l_debug("Found %u frequencies to search for configurator",
> +			l_queue_length(bss_list));
> +
> +	dpp->freqs = scan_freq_set_to_fixed_array(freq_set, &dpp->freqs_len);
> +
> +start:
> +	dpp->current_freq = dpp->freqs[0];

So you'd crash here.

> +
> +	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);
> +
> +	return false;
> +}
> +

Regards,
-Denis