config: read configuration directory "/usr/lib/NetworkManager/conf.d"

This allows packages to install their configuration snippets to
"/usr/", which is a better place for system-provided configuration
files then "/etc".

"/usr/lib/NetworkManager/conf.d/" is read first, so that the values
in /etc have higher priority.

In general, we want to move system-provided configuration away from
/etc, so that a user can do a "factory-reset" by purging /etc.

https://bugzilla.gnome.org/show_bug.cgi?id=738853

contrib/fedora/rpm/NetworkManager.conf

@@ -2,11 +2,21 @@
 #
 # See "man 5 NetworkManager.conf" for details.
 #
+# The directory /usr/lib/NetworkManager/conf.d/ can contain additional configuration
+# snippets installed by packages. These files are read before NetworkManager.conf
+# and have thus lowest priority.
 # The directory /etc/NetworkManager/conf.d/ can contain additional configuration
-# snippets that are installed by some packages. Those snippets override the
+# snippets. Those snippets override the settings from this main file.
-# settings from this main file.
+#
-# To override a configuration from a conf.d/ snippet, add another configuration
+# The files within one conf.d/ directory are read in asciibetical order.
-# with a name sorted lastly (such as 99-my.conf).
+#
+# If /etc/NetworkManager/conf.d/ contains a file with the same name as
+# /usr/lib/NetworkManager/conf.d/, the latter file is shadowed and thus ignored.
+# Hence, to disable loading a file from /usr/lib/NetworkManager/conf.d/ you can
+# put an empty file with the same name.
+#
+# If two files define the same key, the one that is read afterwards will overwrite
+# the previous one.
 
 [main]
 plugins=ifcfg-rh,ibft

contrib/fedora/rpm/NetworkManager.spec

@@ -34,6 +34,7 @@
 
 %define systemd_dir %{_prefix}/lib/systemd/system
 %define udev_dir %{_prefix}/lib/udev
+%define nmlibdir %{_prefix}/lib/%{name}
 
 %global with_adsl 1
 %global with_bluetooth 1
@@ -443,6 +444,7 @@ make install DESTDIR=$RPM_BUILD_ROOT
 %{__cp} %{SOURCE1} $RPM_BUILD_ROOT%{_sysconfdir}/%{name}/
 
 mkdir -p $RPM_BUILD_ROOT%{_sysconfdir}/%{name}/conf.d
+mkdir -p $RPM_BUILD_ROOT%{nmlibdir}/conf.d
 %{__cp} %{SOURCE2} $RPM_BUILD_ROOT%{_sysconfdir}/%{name}/conf.d
 %{__cp} %{SOURCE3} $RPM_BUILD_ROOT%{_sysconfdir}/%{name}/conf.d
 %{__cp} %{SOURCE4} $RPM_BUILD_ROOT%{_sysconfdir}/%{name}/conf.d
@@ -543,6 +545,8 @@ fi
 %endif
 %dir %{_sysconfdir}/%{name}
 %dir %{_sysconfdir}/%{name}/conf.d
+%dir %{nmlibdir}
+%dir %{nmlibdir}/conf.d
 %config %{_sysconfdir}/%{name}/conf.d/10-ibft-plugin.conf
 %{_mandir}/man1/*
 %{_mandir}/man5/*

man/NetworkManager.conf.xml.in

@@ -27,7 +27,8 @@ Copyright 2010 - 2014 Red Hat, Inc.
 
   <refsynopsisdiv>
     <para><filename>/etc/NetworkManager/NetworkManager.conf</filename>,
-    <filename>/etc/NetworkManager/conf.d/<replaceable>name</replaceable>.conf</filename>
+    <filename>/etc/NetworkManager/conf.d/<replaceable>name</replaceable>.conf</filename>,
+    <filename>/usr/lib/NetworkManager/conf.d/<replaceable>name</replaceable>.conf</filename>
     </para>
   </refsynopsisdiv>
 
@@ -42,8 +43,14 @@ Copyright 2010 - 2014 Red Hat, Inc.
     provided by your distribution's packages, you should not modify
     it, since your changes may get overwritten by package
     updates. Instead, you can add additional <literal>.conf</literal>
-    files to the <literal>conf.d</literal> directory. These will be read in order,
+    files to the <literal>/etc/NetworkManager/conf.d</literal> directory.
-    with later files overriding earlier ones.
+    These will be read in order, with later files overriding earlier ones.
+    Packages might install further configuration snippets to <literal>/usr/lib/NetworkManager/conf.d</literal>.
+    This directory is parsed first, even before <literal>NetworkManager.conf</literal>.
+    The loading of a file <literal>/usr/lib/NetworkManager/conf.d/<replaceable>name</replaceable>.conf</literal>
+    can be prevented by adding a file <literal>/etc/NetworkManager/conf.d/<replaceable>name</replaceable>.conf</literal>.
+    In this case, the file from the etc configuration shadows the file from the
+    system configuration directory.
     </para>
 
   </refsect1>

src/Makefile.am

@@ -428,6 +428,7 @@ AM_CPPFLAGS += \
 	-DNMPLUGINDIR=\"$(pkglibdir)\" \
 	-DNMRUNDIR=\"$(nmrundir)\" \
 	-DNMSTATEDIR=\"$(nmstatedir)\" \
+	-DNMLIBDIR=\"$(nmlibdir)\" \
 	\
 	-DDHCLIENT_PATH=\"$(DHCLIENT_PATH)\" \
 	-DDHCPCD_PATH=\"$(DHCPCD_PATH)\" \

src/nm-config.c

@@ -40,11 +40,13 @@
 #define DEFAULT_CONFIG_MAIN_FILE        NMCONFDIR "/NetworkManager.conf"
 #define DEFAULT_CONFIG_DIR              NMCONFDIR "/conf.d"
 #define DEFAULT_CONFIG_MAIN_FILE_OLD    NMCONFDIR "/nm-system-settings.conf"
+#define DEFAULT_SYSTEM_CONFIG_DIR       NMLIBDIR  "/conf.d"
 #define DEFAULT_NO_AUTO_DEFAULT_FILE    NMSTATEDIR "/no-auto-default.state"
 
 struct NMConfigCmdLineOptions {
 	char *config_main_file;
 	char *config_dir;
+	char *system_config_dir;
 	char *no_auto_default_file;
 	char *plugins;
 	gboolean configure_and_quit;
@@ -64,6 +66,7 @@ typedef struct {
 	NMConfigData *config_data_orig;
 
 	char *config_dir;
+	char *system_config_dir;
 	char *no_auto_default_file;
 
 	char **plugins;
@@ -407,6 +410,7 @@ _nm_config_cmd_line_options_clear (NMConfigCmdLineOptions *cli)
 {
 	g_clear_pointer (&cli->config_main_file, g_free);
 	g_clear_pointer (&cli->config_dir, g_free);
+	g_clear_pointer (&cli->system_config_dir, g_free);
 	g_clear_pointer (&cli->no_auto_default_file, g_free);
 	g_clear_pointer (&cli->plugins, g_free);
 	cli->configure_and_quit = FALSE;
@@ -424,6 +428,7 @@ _nm_config_cmd_line_options_copy (const NMConfigCmdLineOptions *cli, NMConfigCmd
 
 	_nm_config_cmd_line_options_clear (dst);
 	dst->config_dir = g_strdup (cli->config_dir);
+	dst->system_config_dir = g_strdup (cli->system_config_dir);
 	dst->config_main_file = g_strdup (cli->config_main_file);
 	dst->no_auto_default_file = g_strdup (cli->no_auto_default_file);
 	dst->plugins = g_strdup (cli->plugins);
@@ -462,6 +467,7 @@ nm_config_cmd_line_options_add_to_entries (NMConfigCmdLineOptions *cli,
 		GOptionEntry config_options[] = {
 			{ "config", 0, 0, G_OPTION_ARG_FILENAME, &cli->config_main_file, N_("Config file location"), N_(DEFAULT_CONFIG_MAIN_FILE) },
 			{ "config-dir", 0, 0, G_OPTION_ARG_FILENAME, &cli->config_dir, N_("Config directory location"), N_(DEFAULT_CONFIG_DIR) },
+			{ "system-config-dir", 0, 0, G_OPTION_ARG_FILENAME, &cli->system_config_dir, N_("System config directory location"), N_(DEFAULT_SYSTEM_CONFIG_DIR) },
 			{ "no-auto-default", 0, G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_FILENAME, &cli->no_auto_default_file, N_("State file for no-auto-default devices"), N_(DEFAULT_NO_AUTO_DEFAULT_FILE) },
 			{ "plugins", 0, 0, G_OPTION_ARG_STRING, &cli->plugins, N_("List of plugins separated by ','"), N_(CONFIG_PLUGINS_DEFAULT) },
 			{ "configure-and-quit", 0, 0, G_OPTION_ARG_NONE, &cli->configure_and_quit, N_("Quit after initial configuration"), NULL },
@@ -550,17 +556,23 @@ _setting_is_string_list (const char *group, const char *key)
 }
 
 static gboolean
-read_config (GKeyFile *keyfile, const char *path, GError **error)
+read_config (GKeyFile *keyfile, const char *dirname, const char *path, GError **error)
 {
 	GKeyFile *kf;
 	char **groups, **keys;
 	gsize ngroups, nkeys;
 	int g, k;
+	gs_free char *path_free = NULL;
 
 	g_return_val_if_fail (keyfile, FALSE);
 	g_return_val_if_fail (path, FALSE);
 	g_return_val_if_fail (!error || !*error, FALSE);
 
+	if (dirname) {
+		path_free = g_build_filename (dirname, path, NULL);
+		path = path_free;
+	}
+
 	if (g_file_test (path, G_FILE_TEST_EXISTS) == FALSE) {
 		g_set_error (error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_NOT_FOUND, "file %s not found", path);
 		return FALSE;
@@ -710,7 +722,7 @@ read_base_config (GKeyFile *keyfile,
 	/* Try a user-specified config file first */
 	if (cli_config_main_file) {
 		/* Bad user-specific config file path is a hard error */
-		if (read_config (keyfile, cli_config_main_file, error)) {
+		if (read_config (keyfile, NULL, cli_config_main_file, error)) {
 			*out_config_main_file = g_strdup (cli_config_main_file);
 			return TRUE;
 		} else
@@ -725,7 +737,7 @@ read_base_config (GKeyFile *keyfile,
 	 */
 
 	/* Try deprecated nm-system-settings.conf first */
-	if (read_config (keyfile, DEFAULT_CONFIG_MAIN_FILE_OLD, &my_error)) {
+	if (read_config (keyfile, NULL, DEFAULT_CONFIG_MAIN_FILE_OLD, &my_error)) {
 		*out_config_main_file = g_strdup (DEFAULT_CONFIG_MAIN_FILE_OLD);
 		return TRUE;
 	}
@@ -738,7 +750,7 @@ read_base_config (GKeyFile *keyfile,
 	g_clear_error (&my_error);
 
 	/* Try the standard config file location next */
-	if (read_config (keyfile, DEFAULT_CONFIG_MAIN_FILE, &my_error)) {
+	if (read_config (keyfile, NULL, DEFAULT_CONFIG_MAIN_FILE, &my_error)) {
 		*out_config_main_file = g_strdup (DEFAULT_CONFIG_MAIN_FILE);
 		return TRUE;
 	}
@@ -771,24 +783,20 @@ sort_asciibetically (gconstpointer a, gconstpointer b)
 }
 
 static GPtrArray *
-_get_config_dir_files (const char *config_main_file,
+_get_config_dir_files (const char *config_dir)
-                       const char *config_dir,
-                       char **out_config_description)
 {
 	GFile *dir;
 	GFileEnumerator *direnum;
 	GFileInfo *info;
 	GPtrArray *confs;
-	GString *config_description;
 	const char *name;
-	guint i;
 
-	g_return_val_if_fail (config_main_file, NULL);
 	g_return_val_if_fail (config_dir, NULL);
-	g_return_val_if_fail (out_config_description && !*out_config_description, NULL);
 
 	confs = g_ptr_array_new_with_free_func (g_free);
-	config_description = g_string_new (config_main_file);
+	if (!*config_dir)
+		return confs;
+
 	dir = g_file_new_for_path (config_dir);
 	direnum = g_file_enumerate_children (dir, G_FILE_ATTRIBUTE_STANDARD_NAME, 0, NULL, NULL);
 	if (direnum) {
@@ -802,43 +810,51 @@ _get_config_dir_files (const char *config_main_file,
 	}
 	g_object_unref (dir);
 
-	if (confs->len > 0) {
+	g_ptr_array_sort (confs, sort_asciibetically);
-		g_ptr_array_sort (confs, sort_asciibetically);
-		g_string_append (config_description, " and conf.d: ");
-		for (i = 0; i < confs->len; i++) {
-			char *n = confs->pdata[i];
-
-			if (i > 0)
-				g_string_append (config_description, ", ");
-			g_string_append (config_description, n);
-			confs->pdata[i] = g_build_filename (config_dir, n, NULL);
-			g_free (n);
-		}
-	}
-
-	*out_config_description = g_string_free (config_description, FALSE);
 	return confs;
 }
 
 static GKeyFile *
 read_entire_config (const NMConfigCmdLineOptions *cli,
                     const char *config_dir,
+                    const char *system_config_dir,
                     char **out_config_main_file,
                     char **out_config_description,
                     GError **error)
 {
 	GKeyFile *keyfile = nm_config_create_keyfile ();
-	GPtrArray *confs;
+	gs_unref_ptrarray GPtrArray *system_confs = NULL;
+	gs_unref_ptrarray GPtrArray *confs = NULL;
 	guint i;
-	char *o_config_main_file = NULL;
+	gs_free char *o_config_main_file = NULL;
-	char *o_config_description = NULL;
 	char **plugins_tmp;
+	GString *str;
 
 	g_return_val_if_fail (config_dir, NULL);
+	g_return_val_if_fail (system_config_dir, NULL);
 	g_return_val_if_fail (out_config_main_file && !*out_config_main_file, FALSE);
 	g_return_val_if_fail (out_config_description && !*out_config_description, NULL);
 	g_return_val_if_fail (!error || !*error, FALSE);
 
+	system_confs = _get_config_dir_files (system_config_dir);
+	confs = _get_config_dir_files (config_dir);
+
+	for (i = 0; i < system_confs->len; ) {
+		const char *filename = system_confs->pdata[i];
+
+		/* if a same named file exists in config_dir, skip it. */
+		if (_nm_utils_strv_find_first ((char **) confs->pdata, confs->len, filename) >= 0) {
+			g_ptr_array_remove_index (system_confs, i);
+			continue;
+		}
+
+		if (!read_config (keyfile, system_config_dir, filename, error)) {
+			g_key_file_free (keyfile);
+			return NULL;
+		}
+		i++;
+	}
+
 	/* First read the base config file */
 	if (!read_base_config (keyfile, cli ? cli->config_main_file : NULL, &o_config_main_file, error)) {
 		g_key_file_free (keyfile);
@@ -847,17 +863,12 @@ read_entire_config (const NMConfigCmdLineOptions *cli,
 
 	g_assert (o_config_main_file);
 
-	confs = _get_config_dir_files (o_config_main_file, config_dir, &o_config_description);
 	for (i = 0; i < confs->len; i++) {
-		if (!read_config (keyfile, confs->pdata[i], error)) {
+		if (!read_config (keyfile, config_dir, confs->pdata[i], error)) {
 			g_key_file_free (keyfile);
-			g_free (o_config_main_file);
-			g_free (o_config_description);
-			g_ptr_array_unref (confs);
 			return NULL;
 		}
 	}
-	g_ptr_array_unref (confs);
 
 	/* Merge settings from command line. They overwrite everything read from
 	 * config files. */
@@ -881,8 +892,32 @@ read_entire_config (const NMConfigCmdLineOptions *cli,
 	if (cli && cli->connectivity_response && cli->connectivity_response[0])
 		g_key_file_set_string (keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "response", cli->connectivity_response);
 
+	str = g_string_new (o_config_main_file);
+	if (system_confs->len > 0) {
+		for (i = 0; i < system_confs->len; i++) {
+			if (i == 0)
+				g_string_append (str, " (lib: ");
+			else
+				g_string_append (str, ", ");
+			g_string_append (str, system_confs->pdata[i]);
+		}
+		g_string_append (str, ")");
+	}
+	if (confs->len > 0) {
+		for (i = 0; i < confs->len; i++) {
+			if (i == 0)
+				g_string_append (str, " (etc: ");
+			else
+				g_string_append (str, ", ");
+			g_string_append (str, confs->pdata[i]);
+		}
+		g_string_append (str, ")");
+	}
+
 	*out_config_main_file = o_config_main_file;
-	*out_config_description = o_config_description;
+	*out_config_description = g_string_free (str, FALSE);
+
+	o_config_main_file = NULL;
 	return keyfile;
 }
 
@@ -928,6 +963,7 @@ nm_config_reload (NMConfig *self, int signal)
 	 */
 	keyfile = read_entire_config (&priv->cli,
 	                              priv->config_dir,
+	                              priv->system_config_dir,
 	                              &config_main_file,
 	                              &config_description,
 	                              &error);
@@ -1087,8 +1123,21 @@ init_sync (GInitable *initable, GCancellable *cancellable, GError **error)
 	else
 		priv->config_dir = g_strdup (DEFAULT_CONFIG_DIR);
 
+	if (priv->cli.system_config_dir)
+		priv->system_config_dir = g_strdup (priv->cli.system_config_dir);
+	else
+		priv->system_config_dir = g_strdup (DEFAULT_SYSTEM_CONFIG_DIR);
+
+	if (strcmp (priv->config_dir, priv->system_config_dir) == 0) {
+		/* having the same directory twice makes no sense. In that case, clear
+		 * @system_config_dir. */
+		g_free (priv->system_config_dir);
+		priv->system_config_dir = g_strdup ("");
+	}
+
 	keyfile = read_entire_config (&priv->cli,
 	                              priv->config_dir,
+	                              priv->system_config_dir,
 	                              &config_main_file,
 	                              &config_description,
 	                              error);
@@ -1156,6 +1205,7 @@ finalize (GObject *gobject)
 	NMConfigPrivate *priv = NM_CONFIG_GET_PRIVATE (gobject);
 
 	g_free (priv->config_dir);
+	g_free (priv->system_config_dir);
 	g_free (priv->no_auto_default_file);
 	g_strfreev (priv->plugins);
 	g_free (priv->dhcp_client);

src/tests/config/test-config.c

@@ -33,7 +33,7 @@
 #include "nm-test-utils.h"
 
 static NMConfig *
-setup_config (GError **error, const char *config_file, const char *config_dir, ...)
+setup_config (GError **error, const char *config_file, const char *config_dir, const char *system_config_dir, ...)
 {
 	va_list ap;
 	GPtrArray *args;
@@ -53,8 +53,12 @@ setup_config (GError **error, const char *config_file, const char *config_dir, .
 	g_ptr_array_add (args, (char *)config_file);
 	g_ptr_array_add (args, "--config-dir");
 	g_ptr_array_add (args, (char *)config_dir);
+	if (system_config_dir) {
+		g_ptr_array_add (args, "--system-config-dir");
+		g_ptr_array_add (args, (char *) system_config_dir);
+	}
 
-	va_start (ap, config_dir);
+	va_start (ap, system_config_dir);
 	while ((arg = va_arg (ap, char *)))
 		g_ptr_array_add (args, arg);
 	va_end (ap);
@@ -97,7 +101,7 @@ test_config_simple (void)
 	gs_unref_object NMDevice *dev51 = nm_test_device_new ("00:00:00:00:00:51");
 	gs_unref_object NMDevice *dev52 = nm_test_device_new ("00:00:00:00:00:52");
 
-	config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "/no/such/dir", NULL);
+	config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "/no/such/dir", "", NULL);
 
 	g_assert_cmpstr (nm_config_data_get_config_main_file (nm_config_get_data_orig (config)), ==, SRCDIR "/NetworkManager.conf");
 	g_assert_cmpstr (nm_config_get_dhcp_client (config), ==, "dhclient");
@@ -176,7 +180,7 @@ test_config_non_existent (void)
 {
 	GError *error = NULL;
 
-	setup_config (&error, SRCDIR "/no-such-file", "/no/such/dir", NULL);
+	setup_config (&error, SRCDIR "/no-such-file", "/no/such/dir", "", NULL);
 	g_assert_error (error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_NOT_FOUND);
 	g_clear_error (&error);
 }
@@ -186,7 +190,7 @@ test_config_parse_error (void)
 {
 	GError *error = NULL;
 
-	setup_config (&error, SRCDIR "/bad.conf", "/no/such/dir", NULL);
+	setup_config (&error, SRCDIR "/bad.conf", "/no/such/dir", "", NULL);
 	g_assert_error (error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_PARSE);
 	g_clear_error (&error);
 }
@@ -197,7 +201,7 @@ test_config_override (void)
 	NMConfig *config;
 	const char **plugins;
 
-	config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "/no/such/dir",
+	config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "/no/such/dir", "",
 	                       "--plugins", "alpha,beta,gamma,delta",
 	                       "--connectivity-interval", "12",
 	                       NULL);
@@ -235,7 +239,7 @@ test_config_no_auto_default (void)
 	g_assert_cmpint (nwrote, ==, 18);
 	close (fd);
 
-	config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "/no/such/dir",
+	config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "/no/such/dir", "",
 	                       "--no-auto-default", state_file,
 	                       NULL);
 
@@ -257,7 +261,7 @@ test_config_no_auto_default (void)
 
 	g_object_unref (config);
 
-	config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "/no/such/dir",
+	config = setup_config (NULL, SRCDIR "/NetworkManager.conf", "/no/such/dir", "",
 	                       "--no-auto-default", state_file,
 	                       NULL);
 
@@ -285,7 +289,7 @@ test_config_confdir (void)
 	char *value;
 	GSList *specs;
 
-	config = setup_config (NULL, SRCDIR "/NetworkManager.conf", SRCDIR "/conf.d", NULL);
+	config = setup_config (NULL, SRCDIR "/NetworkManager.conf", SRCDIR "/conf.d", "", NULL);
 
 	g_assert_cmpstr (nm_config_data_get_config_main_file (nm_config_get_data_orig (config)), ==, SRCDIR "/NetworkManager.conf");
 	g_assert_cmpstr (nm_config_get_dhcp_client (config), ==, "dhcpcd");
@@ -391,7 +395,7 @@ test_config_confdir_parse_error (void)
 	GError *error = NULL;
 
 	/* Using SRCDIR as the conf dir will pick up bad.conf */
-	setup_config (&error, SRCDIR "/NetworkManager.conf", SRCDIR, NULL);
+	setup_config (&error, SRCDIR "/NetworkManager.conf", SRCDIR, "", NULL);
 	g_assert_error (error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_PARSE);
 	g_clear_error (&error);
 }