diff mbox series

[2/5] knownnetworks: network: support updating known network settings

Message ID 20231214180110.130991-2-prestwoj@gmail.com (mailing list archive)
State New
Headers show
Series [1/5] network: remove 'path' from settings_load_pt_ecc | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
prestwoj/iwd-ci-gitlint success GitLint

Commit Message

James Prestwood Dec. 14, 2023, 6:01 p.m. UTC 
Currently if a known network is modified on disk the settings are not
reloaded by network. Only disconnecting/reconnecting to the network
would update the settings. This poses an issue to DPP since its
creating or updating a known network after configuration then trying
to connect. The connection itself works fine since the PSK/passphrase
is set to the network object directly, but any additional settings
are not updated.

To fix this add a new UPDATED known network event. This is then
handled from within network and all settings read from disk are
applied to the network object.
---
 src/knownnetworks.c |   4 ++
 src/knownnetworks.h |   1 +
 src/network.c       | 104 ++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 106 insertions(+), 3 deletions(-)

Comments

Denis Kenzior Dec. 15, 2023, 4:37 p.m. UTC | #1 
Hi James,

On 12/14/23 12:01, James Prestwood wrote:
> Currently if a known network is modified on disk the settings are not
> reloaded by network. Only disconnecting/reconnecting to the network
> would update the settings. This poses an issue to DPP since its
> creating or updating a known network after configuration then trying
> to connect. The connection itself works fine since the PSK/passphrase
> is set to the network object directly, but any additional settings
> are not updated.
> 
> To fix this add a new UPDATED known network event. This is then
> handled from within network and all settings read from disk are
> applied to the network object.

Okay, so you chose option 2...

> ---
>   src/knownnetworks.c |   4 ++
>   src/knownnetworks.h |   1 +
>   src/network.c       | 104 ++++++++++++++++++++++++++++++++++++++++++--
>   3 files changed, 106 insertions(+), 3 deletions(-)
> 

<snip>

> diff --git a/src/network.c b/src/network.c
> index 41c5460b..bd3671ca 100644
> --- a/src/network.c
> +++ b/src/network.c
> @@ -730,6 +730,73 @@ static void network_settings_save(struct network *network,
>   		network_settings_save_sae_pt_ecc(settings, network->sae_pt_20);
>   }
>   
> +static bool network_settings_update(struct network *network,
> +					struct l_settings *new)
> +{
> +	bool have_transition_disable;
> +	uint8_t transition_disable = 0;
> +	unsigned int i;
> +	size_t psk_len;
> +	_auto_(l_strv_free) char **list = NULL;
> +	_auto_(l_free) uint8_t *psk = NULL;
> +	_auto_(l_free) char *passphrase = NULL;
> +
> +	if (l_settings_get_bool(new, NET_TRANSITION_DISABLE,
> +				&have_transition_disable) &&
> +				have_transition_disable) {
> +		list = l_settings_get_string_list(new,
> +					NET_TRANSITION_DISABLE_MODES, ' ');
> +
> +		for (i = 0; list[i]; i++) {
> +			if (!strcmp(list[i], "personal"))
> +				set_bit(&transition_disable, 0);
> +			else if (!strcmp(list[i], "enterprise"))
> +				set_bit(&transition_disable, 1);
> +			else if (!strcmp(list[i], "open"))
> +				set_bit(&transition_disable, 2);
> +		}
> +
> +		have_transition_disable = true;
> +	} else
> +		have_transition_disable = false;
> +
> +	if (network->security != SECURITY_PSK)
> +		goto apply;
> +
> +	psk = l_settings_get_bytes(network->settings, "Security",
> +					"PreSharedKey", &psk_len);
> +	if (psk && psk_len != 32) {
> +		l_warn("updated [Security].PreSharedKey value is invalid!");
> +		return false;
> +	}

I'm not sure you really want to yank the PSK or Passphrase actually.  These are 
obtained via the Agent and should take precedence.

> +
> +	passphrase = l_settings_get_string(network->settings,
> +						"Security", "Passphrase");
> +	if (passphrase && !crypto_passphrase_is_valid(passphrase)) {
> +		l_warn("updated [Security].Passphrase value is invalid!");
> +		return false;
> +	}
> +
> +apply:
> +	network_settings_close(network);
> +	network->settings = new;
> +
> +	network->have_transition_disable = have_transition_disable;
> +	network->transition_disable = transition_disable;
> +
> +	if (psk)
> +		network->psk = l_steal_ptr(psk);
> +
> +	if (passphrase) {
> +		network->passphrase = l_strdup(passphrase);
> +
> +		network_settings_load_pt_ecc(network, 19, &network->sae_pt_19);
> +		network_settings_load_pt_ecc(network, 20, &network->sae_pt_20);
> +	}

I suspect you're better off copying all the settings (with the exception of 
Security) over to the open settings object instead,

> +
> +	return true;
> +}
> +
>   void network_sync_settings(struct network *network)
>   {
>   	struct network_info *info = network->info;
> @@ -1966,17 +2033,32 @@ static void network_update_hotspot(struct network *network, void *user_data)
>   	match_hotspot_network(info, network);
>   }
>   
> -static void match_known_network(struct station *station, void *user_data)
> +static void match_known_network(struct station *station, void *user_data,
> +				bool new)
>   {
>   	struct network_info *info = user_data;
>   	struct network *network;
>   
>   	if (!info->is_hotspot) {
> +		struct l_settings *settings;
>   		network = station_network_find(station, info->ssid, info->type);
>   		if (!network)
>   			return;
>   
> -		network_set_info(network, info);
> +		/* New networks should load settings upon connecting */
> +		if (new) {
> +			network_set_info(network, info);
> +			return;
> +		}

Don't you want to update the settings even for the EVENT_ADDED case?  Something 
like this might happen for example:

- User issues Network.Connect() for an unknown network
- At the same time, the network file is written to /var/lib/iwd

Connect proceeds to win the race, with the EVENT_ADDED event coming in shortly 
after.  Don't you want any additional settings (say DHCP hostname or whatever) 
to be used when the network succeeds in associating?

> +
> +		settings = network_info_open_settings(info);
> +
> +		if (!settings || !network_settings_update(network, settings)) {
> +			l_warn("Failed to apply new/updated settings (%s)",
> +					info->ssid);
> +			l_settings_free(settings);
> +		}
> +
>   		return;
>   	}
>   

<snip>

> +	case KNOWN_NETWORKS_EVENT_UPDATED:
> +		station_foreach(update_known_network, (void *) info);
> +
> +		/* Syncs frequencies of updated known network */
> +		known_network_frequency_sync((struct network_info *)info);
> +		break;

Hmm, why do this on the update?

>   	case KNOWN_NETWORKS_EVENT_REMOVED:
>   		station_foreach(emit_known_network_removed, (void *) info);
>   		break;

Regards,
-Denis
diff mbox series

Patch

diff --git a/src/knownnetworks.c b/src/knownnetworks.c
index d4d50a6f..04ce74ec 100644
--- a/src/knownnetworks.c
+++ b/src/knownnetworks.c
@@ -468,6 +468,10 @@  void known_network_update(struct network_info *network,
 	known_network_set_autoconnect(network, new->is_autoconnectable);
 
 	memcpy(&network->config, new, sizeof(struct network_config));
+
+	WATCHLIST_NOTIFY(&known_network_watches,
+				known_networks_watch_func_t,
+				KNOWN_NETWORKS_EVENT_UPDATED, network);
 }
 
 bool known_networks_foreach(known_networks_foreach_func_t function,
diff --git a/src/knownnetworks.h b/src/knownnetworks.h
index 0a5c9e25..e8ffac0b 100644
--- a/src/knownnetworks.h
+++ b/src/knownnetworks.h
@@ -35,6 +35,7 @@  struct network_info;
 enum known_networks_event {
 	KNOWN_NETWORKS_EVENT_ADDED,
 	KNOWN_NETWORKS_EVENT_REMOVED,
+	KNOWN_NETWORKS_EVENT_UPDATED,
 };
 
 struct network_info_ops {
diff --git a/src/network.c b/src/network.c
index 41c5460b..bd3671ca 100644
--- a/src/network.c
+++ b/src/network.c
@@ -730,6 +730,73 @@  static void network_settings_save(struct network *network,
 		network_settings_save_sae_pt_ecc(settings, network->sae_pt_20);
 }
 
+static bool network_settings_update(struct network *network,
+					struct l_settings *new)
+{
+	bool have_transition_disable;
+	uint8_t transition_disable = 0;
+	unsigned int i;
+	size_t psk_len;
+	_auto_(l_strv_free) char **list = NULL;
+	_auto_(l_free) uint8_t *psk = NULL;
+	_auto_(l_free) char *passphrase = NULL;
+
+	if (l_settings_get_bool(new, NET_TRANSITION_DISABLE,
+				&have_transition_disable) &&
+				have_transition_disable) {
+		list = l_settings_get_string_list(new,
+					NET_TRANSITION_DISABLE_MODES, ' ');
+
+		for (i = 0; list[i]; i++) {
+			if (!strcmp(list[i], "personal"))
+				set_bit(&transition_disable, 0);
+			else if (!strcmp(list[i], "enterprise"))
+				set_bit(&transition_disable, 1);
+			else if (!strcmp(list[i], "open"))
+				set_bit(&transition_disable, 2);
+		}
+
+		have_transition_disable = true;
+	} else
+		have_transition_disable = false;
+
+	if (network->security != SECURITY_PSK)
+		goto apply;
+
+	psk = l_settings_get_bytes(network->settings, "Security",
+					"PreSharedKey", &psk_len);
+	if (psk && psk_len != 32) {
+		l_warn("updated [Security].PreSharedKey value is invalid!");
+		return false;
+	}
+
+	passphrase = l_settings_get_string(network->settings,
+						"Security", "Passphrase");
+	if (passphrase && !crypto_passphrase_is_valid(passphrase)) {
+		l_warn("updated [Security].Passphrase value is invalid!");
+		return false;
+	}
+
+apply:
+	network_settings_close(network);
+	network->settings = new;
+
+	network->have_transition_disable = have_transition_disable;
+	network->transition_disable = transition_disable;
+
+	if (psk)
+		network->psk = l_steal_ptr(psk);
+
+	if (passphrase) {
+		network->passphrase = l_strdup(passphrase);
+
+		network_settings_load_pt_ecc(network, 19, &network->sae_pt_19);
+		network_settings_load_pt_ecc(network, 20, &network->sae_pt_20);
+	}
+
+	return true;
+}
+
 void network_sync_settings(struct network *network)
 {
 	struct network_info *info = network->info;
@@ -1966,17 +2033,32 @@  static void network_update_hotspot(struct network *network, void *user_data)
 	match_hotspot_network(info, network);
 }
 
-static void match_known_network(struct station *station, void *user_data)
+static void match_known_network(struct station *station, void *user_data,
+				bool new)
 {
 	struct network_info *info = user_data;
 	struct network *network;
 
 	if (!info->is_hotspot) {
+		struct l_settings *settings;
 		network = station_network_find(station, info->ssid, info->type);
 		if (!network)
 			return;
 
-		network_set_info(network, info);
+		/* New networks should load settings upon connecting */
+		if (new) {
+			network_set_info(network, info);
+			return;
+		}
+
+		settings = network_info_open_settings(info);
+
+		if (!settings || !network_settings_update(network, settings)) {
+			l_warn("Failed to apply new/updated settings (%s)",
+					info->ssid);
+			l_settings_free(settings);
+		}
+
 		return;
 	}
 
@@ -1984,17 +2066,33 @@  static void match_known_network(struct station *station, void *user_data)
 	station_network_foreach(station, network_update_hotspot, info);
 }
 
+static void add_known_network(struct station *station, void *user_data)
+{
+	match_known_network(station, (struct network_info *)user_data, true);
+}
+
+static void update_known_network(struct station *station, void *user_data)
+{
+	match_known_network(station, (struct network_info *)user_data, false);
+}
+
 static void known_networks_changed(enum known_networks_event event,
 					const struct network_info *info,
 					void *user_data)
 {
 	switch (event) {
 	case KNOWN_NETWORKS_EVENT_ADDED:
-		station_foreach(match_known_network, (void *) info);
+		station_foreach(add_known_network, (void *) info);
 
 		/* Syncs frequencies of newly known network */
 		known_network_frequency_sync((struct network_info *)info);
 		break;
+	case KNOWN_NETWORKS_EVENT_UPDATED:
+		station_foreach(update_known_network, (void *) info);
+
+		/* Syncs frequencies of updated known network */
+		known_network_frequency_sync((struct network_info *)info);
+		break;
 	case KNOWN_NETWORKS_EVENT_REMOVED:
 		station_foreach(emit_known_network_removed, (void *) info);
 		break;