diff mbox series

Implement read-only mode for SSIDs

Message ID 20210505135343.czilnrvptdhiv2vs@topsnens
State New
Headers show
Series Implement read-only mode for SSIDs | expand

Commit Message

Maximilian Bosch May 5, 2021, 1:53 p.m. UTC
On NixOS[1] - a Linux distribution which allows to configure a full OS
declaratively - it's possible to configure SSIDs for `wpa_supplicant`
like this:

    networking.wireless.networks = {
      myssid = {
        pskRaw = "<redacted>";
      };
    };

It's also possible to add networks "imperatively" using `wpa_gui` or
`wpa_cli`. However it's not possible to do both because if the first
option is used, NixOS creates a read-only symlink at
`/etc/wpa_supplicant.conf` and then it's not possible for
`wpa_supplicant` anymore to write to it.

This patch aims to help us changing this: while "declarative" SSID
configuration can be quite useful, it's a bad idea for e.g. sensitive
stuff like a WPA2 enterprise network.

The original idea was to use `-I`[2] for immutable configs (including
"declarative" networks) on NixOS and `-c /etc/wpa_supplicant.conf` for
anything "imperative".

However this doesn't really work out because if a wifi network from a
config file specified with `-I` is changed by e.g. `wpa_gui`, it's
silently overwritten in `/etc/wpa_supplicant.conf` (specified with
`-c`) which is IMHO unintuitive (in our case at least). This patch
basically declares each network defined in a config file passed via `-I`
to `wpa_supplicant` as "read-only" and doesn't write these "read-only"
networks to `/etc/wpa_supplicant.conf`.

A bit more context can be found on GitHub in the PR where I implemented
this[3].

I know that this is a fairly distro-specific thing and has a rather low
chance to actually land in upstream because of that. However I figured
that it's still worth a try, especially because `-I` seems to be
designed for "global" configuration options only and its behavior if
SSIDs are defined in there doesn't seem explicitly defined to me.

Last but not least, I'm not a subscriber of this mailing list so while
reviewing this, it'd be good if you could leave my email explicitly in
the `To:`-section.

[1] https://nixos.org/
[2] Added in e6304cad47251e88d073553042f1ea7805a858d1
[3] https://github.com/NixOS/nixpkgs/pull/113716

Signed-off-by: Maximilian Bosch <maximilian@mbosch.me>
---
 wpa_supplicant/config.h         |  2 +-
 wpa_supplicant/config_file.c    |  5 +++--
 wpa_supplicant/config_none.c    |  2 +-
 wpa_supplicant/config_ssid.h    | 15 +++++++++++++++
 wpa_supplicant/wpa_supplicant.c |  8 ++++----
 5 files changed, 24 insertions(+), 8 deletions(-)
diff mbox series

Patch

diff --git a/wpa_supplicant/config.h b/wpa_supplicant/config.h
index 68679c6e3..8e0a692c5 100644
--- a/wpa_supplicant/config.h
+++ b/wpa_supplicant/config.h
@@ -1731,7 +1731,7 @@  const char * wpa_config_get_global_field_name(unsigned int i, int *no_var);
  *
  * Each configuration backend needs to implement this function.
  */
-struct wpa_config * wpa_config_read(const char *name, struct wpa_config *cfgp);
+struct wpa_config * wpa_config_read(const char *name, struct wpa_config *cfgp, int ro);
 
 /**
  * wpa_config_write - Write or update configuration data
diff --git a/wpa_supplicant/config_file.c b/wpa_supplicant/config_file.c
index a535e3f08..0ad5594fc 100644
--- a/wpa_supplicant/config_file.c
+++ b/wpa_supplicant/config_file.c
@@ -289,7 +289,7 @@  static int wpa_config_process_blob(struct wpa_config *config, FILE *f,
 #endif /* CONFIG_NO_CONFIG_BLOBS */
 
 
-struct wpa_config * wpa_config_read(const char *name, struct wpa_config *cfgp)
+struct wpa_config * wpa_config_read(const char *name, struct wpa_config *cfgp, int ro)
 {
 	FILE *f;
 	char buf[512], *pos;
@@ -331,6 +331,7 @@  struct wpa_config * wpa_config_read(const char *name, struct wpa_config *cfgp)
 	while (wpa_config_get_line(buf, sizeof(buf), f, &line, &pos)) {
 		if (os_strcmp(pos, "network={") == 0) {
 			ssid = wpa_config_read_network(f, &line, id++);
+			ssid->ro = ro;
 			if (ssid == NULL) {
 				wpa_printf(MSG_ERROR, "Line %d: failed to "
 					   "parse network block.", line);
@@ -1581,7 +1582,7 @@  int wpa_config_write(const char *name, struct wpa_config *config)
 	}
 
 	for (ssid = config->ssid; ssid; ssid = ssid->next) {
-		if (ssid->key_mgmt == WPA_KEY_MGMT_WPS || ssid->temporary)
+		if (ssid->key_mgmt == WPA_KEY_MGMT_WPS || ssid->temporary || ssid->ro)
 			continue; /* do not save temporary networks */
 		if (wpa_key_mgmt_wpa_psk_no_sae(ssid->key_mgmt) &&
 		    !ssid->psk_set && !ssid->passphrase)
diff --git a/wpa_supplicant/config_none.c b/wpa_supplicant/config_none.c
index 2aac28fa3..02191b425 100644
--- a/wpa_supplicant/config_none.c
+++ b/wpa_supplicant/config_none.c
@@ -17,7 +17,7 @@ 
 #include "base64.h"
 
 
-struct wpa_config * wpa_config_read(const char *name, struct wpa_config *cfgp)
+struct wpa_config * wpa_config_read(const char *name, struct wpa_config *cfgp, int ro)
 {
 	struct wpa_config *config;
 
diff --git a/wpa_supplicant/config_ssid.h b/wpa_supplicant/config_ssid.h
index 3f7b31480..73f113391 100644
--- a/wpa_supplicant/config_ssid.h
+++ b/wpa_supplicant/config_ssid.h
@@ -104,6 +104,21 @@  struct wpa_ssid {
 	 */
 	int id;
 
+	/**
+	 * ro - whether a network is declared as read-only
+	 *
+	 * Every network which is defined in a config file that is passed to
+	 * `wpa_supplicant` using the `-I`-option will be marked as read-only
+	 * using this flag. It has the effect that it won't be written to
+	 * `/etc/wpa_supplicant.conf` if e.g. `wpa_gui` tells the daemon to
+	 * save all changed configs.
+	 *
+	 * This is necessary because networks from `/etc/wpa_supplicant.conf`
+	 * have a higher priority and changes from an alternative file would be
+	 * silently overwritten without this.
+	 */
+	int ro;
+
 	/**
 	 * priority - Priority group
 	 *
diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
index 835b33575..278e8b8c2 100644
--- a/wpa_supplicant/wpa_supplicant.c
+++ b/wpa_supplicant/wpa_supplicant.c
@@ -1137,14 +1137,14 @@  int wpa_supplicant_reload_configuration(struct wpa_supplicant *wpa_s)
 
 	if (wpa_s->confname == NULL)
 		return -1;
-	conf = wpa_config_read(wpa_s->confname, NULL);
+	conf = wpa_config_read(wpa_s->confname, NULL, 0);
 	if (conf == NULL) {
 		wpa_msg(wpa_s, MSG_ERROR, "Failed to parse the configuration "
 			"file '%s' - exiting", wpa_s->confname);
 		return -1;
 	}
 	if (wpa_s->confanother &&
-	    !wpa_config_read(wpa_s->confanother, conf)) {
+	    !wpa_config_read(wpa_s->confanother, conf, 1)) {
 		wpa_msg(wpa_s, MSG_ERROR,
 			"Failed to parse the configuration file '%s' - exiting",
 			wpa_s->confanother);
@@ -6342,7 +6342,7 @@  static int wpa_supplicant_init_iface(struct wpa_supplicant *wpa_s,
 #else /* CONFIG_BACKEND_FILE */
 		wpa_s->confname = os_strdup(iface->confname);
 #endif /* CONFIG_BACKEND_FILE */
-		wpa_s->conf = wpa_config_read(wpa_s->confname, NULL);
+		wpa_s->conf = wpa_config_read(wpa_s->confname, NULL, 0);
 		if (wpa_s->conf == NULL) {
 			wpa_printf(MSG_ERROR, "Failed to read or parse "
 				   "configuration '%s'.", wpa_s->confname);
@@ -6350,7 +6350,7 @@  static int wpa_supplicant_init_iface(struct wpa_supplicant *wpa_s,
 		}
 		wpa_s->confanother = os_rel2abs_path(iface->confanother);
 		if (wpa_s->confanother &&
-		    !wpa_config_read(wpa_s->confanother, wpa_s->conf)) {
+		    !wpa_config_read(wpa_s->confanother, wpa_s->conf, 1)) {
 			wpa_printf(MSG_ERROR,
 				   "Failed to read or parse configuration '%s'.",
 				   wpa_s->confanother);