device: remove reading ipv6 privacy setting from sysctlth/connection-defaults-bgo695383

Since introduction for support of ip6-privacy (use_tempaddr,
RFC4941) with commit d376270bfe673c041e610a981bd6c77c7cb37ba1,
the sysctl value from /etc was always read first.

This is problematic, because an explicit setting in the
connection should not be ignored over a global configuration.

Drop that old behavior. It was also problematic, because we did
not read any files under /etc/sysctl.d (except for sysctl.conf).
Also, we did not honor per-interface configurations.

Now we also use as last fallback the value from
/proc/sys/net/ipv6/conf/default/use_tempaddr
That has the advantage of falling back to the system default value
so that NM doesn't need to have it's own default policy
(Related: https://bugzilla.redhat.com/show_bug.cgi?id=1187525).

This is a change in behavior.

3 files changed, 9 insertions, 83 deletions

diff --git a/libnm-core/nm-setting-ip6-config.c b/libnm-core/nm-setting-ip6-config.c
index 1bb40f0ed5..7e3ae4d4da 100644
--- a/libnm-core/nm-setting-ip6-config.c
+++ b/libnm-core/nm-setting-ip6-config.c
@@ -508,19 +508,10 @@ nm_setting_ip6_config_class_init (NMSettingIP6ConfigClass *ip6_class)
 	 * 0: disabled, 1: enabled (prefer public address), 2: enabled (prefer temporary
 	 * addresses).
 	 *
-	 * This property can be configured with a default value in global configuration
-	 * NetworkManager.conf.
+	 * Having a per-connection setting set to "-1" (unknown) means fallback to
+	 * global configuration "ipv6.ip6-privacy".
 	 *
-	 * If the global configuration value "connection.ipv6.ip6-privacy"
-	 * is not specified, the sysctl value "net.ipv6.conf.default.use_tempaddr" in /etc/sysctl.conf or
-	 * /lib/sysctl.d/sysctl.conf is always checked first. If set to "0", "1", or "2", that
-	 * value is always used and any per-connection setting is ignored. This behavior is kept for
-	 * backward compatiblity.
-	 *
-	 * Otherwise this per-connection setting is honored next. Having a per-connection setting set
-	 * to "-1" (unknown) means fallback to global configuration "ipv6.ip6-privacy".
-	 *
-	 * If the global configuration is explicitly set to "-1", fallback to read
+	 * If also global configuration is unspecified or set to "-1", fallback to read
 	 * "/proc/sys/net/ipv6/conf/default/use_tempaddr".
 	 **/
 	/* ---ifcfg-rh---
diff --git a/man/NetworkManager.conf.xml.in b/man/NetworkManager.conf.xml.in
index fe32515c22..dc3069470e 100644
--- a/man/NetworkManager.conf.xml.in
+++ b/man/NetworkManager.conf.xml.in
@@ -524,10 +524,7 @@ ipv6.ip6-privacy=1
         </varlistentry>
         <varlistentry>
           <term><varname>ipv6.ip6-privacy</varname></term>
-          <listitem><para>If this value is unset, NetworkManager will always first check "/etc/sysctl.conf" and "/etc/sysctl.d/sysctl.conf" whether
-            they contain "net.ipv6.conf.default.use_tempaddr". This value is then preferred over any per-connection
-            setting. That step is omitted when setting the global configuration value <literal>ipv6.ip6-privacy</literal>
-            to any value. If <literal>ipv6.ip6-privacy</literal> is set but neither "0", "1", or "2", use the content of
+          <listitem><para>If <literal>ipv6.ip6-privacy</literal> is unset, use the content of
             "/proc/sys/net/ipv6/conf/default/use_tempaddr" as last fallback.
           </para></listitem>
         </varlistentry>
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index 5132c5a53b..ee767ebcab 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -4726,45 +4726,6 @@ _ip6_privacy_clamp (NMSettingIP6ConfigPrivacy use_tempaddr)
 	}
 }
 
-/* Get net.ipv6.conf.default.use_tempaddr value from /etc/sysctl.conf or
- * /lib/sysctl.d/sysctl.conf
- */
-static NMSettingIP6ConfigPrivacy
-_ip6_privacy_sysctl (void)
-{
-	char *contents = NULL;
-	const char *group_name = "[forged_group]\n";
-	char *sysctl_data = NULL;
-	GKeyFile *keyfile;
-	GError *error = NULL;
-	gint tmp;
-	NMSettingIP6ConfigPrivacy ret = NM_SETTING_IP6_CONFIG_PRIVACY_UNKNOWN;
-
-	/* Read file contents to a string. */
-	if (!g_file_get_contents ("/etc/sysctl.conf", &contents, NULL, NULL))
-		if (!g_file_get_contents ("/lib/sysctl.d/sysctl.conf", &contents, NULL, NULL))
-			return NM_SETTING_IP6_CONFIG_PRIVACY_UNKNOWN;
-
-	/* Prepend a group so that we can use GKeyFile parser. */
-	sysctl_data = g_strdup_printf ("%s%s", group_name, contents);
-
-	keyfile = g_key_file_new ();
-	if (!g_key_file_load_from_data (keyfile, sysctl_data, -1, G_KEY_FILE_NONE, NULL))
-		goto done;
-
-	tmp = g_key_file_get_integer (keyfile, "forged_group", "net.ipv6.conf.default.use_tempaddr", &error);
-	if (error == NULL)
-		ret = _ip6_privacy_clamp (tmp);
-
-done:
-	g_free (contents);
-	g_free (sysctl_data);
-	g_clear_error (&error);
-	g_key_file_free (keyfile);
-
-	return ret;
-}
-
 static NMSettingIP6ConfigPrivacy
 _ip6_privacy_get (NMDevice *self)
 {
@@ -4774,20 +4735,7 @@ _ip6_privacy_get (NMDevice *self)
 
 	g_return_val_if_fail (self, NM_SETTING_IP6_CONFIG_PRIVACY_UNKNOWN);
 
-	value = nm_config_data_get_connection_default (nm_config_get_data (nm_config_get ()),
-	                                               "ipv6.ip6-privacy", self);
-
-	/* 1.) If (and only if) the default value is not configured, check _ip6_privacy_sysctl()
-	 * first. This is to preserve backward compatibility. In this case -- having no
-	 * default value in global configuration, but use_tempaddr configured in /etc/sysctl --
-	 * the per-connection setting is always ignored. */
-	if (!value) {
-		ip6_privacy = _ip6_privacy_sysctl ();
-		if (ip6_privacy != NM_SETTING_IP6_CONFIG_PRIVACY_UNKNOWN)
-			return ip6_privacy;
-	}
-
-	/* 2.) Next we always look at the per-connection setting. If it is not -1 (unknown),
+	/* 1.) First look at the per-connection setting. If it is not -1 (unknown),
 	 * use it. */
 	connection = nm_device_get_connection (self);
 	if (connection) {
@@ -4801,13 +4749,10 @@ _ip6_privacy_get (NMDevice *self)
 		}
 	}
 
-	/* 3.) All options (per-connection, global, sysctl) are unset/default.
-	 * Return UNKNOWN. Skip step 5.) because that would be a change in behavior
-	 * compared to older versions. */
-	if (!value)
-		return NM_SETTING_IP6_CONFIG_PRIVACY_UNKNOWN;
+	value = nm_config_data_get_connection_default (nm_config_get_data (nm_config_get ()),
+	                                               "ipv6.ip6-privacy", self);
 
-	/* 4.) use the default value from the configuration. */
+	/* 2.) use the default value from the configuration. */
 	ip6_privacy = _nm_utils_ascii_str_to_int64 (value, 10,
 	                                            NM_SETTING_IP6_CONFIG_PRIVACY_UNKNOWN,
 	                                            NM_SETTING_IP6_CONFIG_PRIVACY_PREFER_TEMP_ADDR,
@@ -4815,15 +4760,8 @@ _ip6_privacy_get (NMDevice *self)
 	if (ip6_privacy != NM_SETTING_IP6_CONFIG_PRIVACY_UNKNOWN)
 		return ip6_privacy;
 
-	/* 5.) A default-value is configured, but it is invalid/unknown. Fallback to sysctl reading.
-	 *
-	 * _ip6_privacy_sysctl() only reads two files from /etc and does not support the complexity
-	 * of parsing all files. Also, it only considers "net.ipv6.conf.default.use_tempaddr",
-	 * not the per-interface values. This is kinda unexpected, but we do it in 1.) to preserve
-	 * old behavior.
+	/* 3.) No valid default-value configured. Fallback to reading sysctl.
 	 *
-	 * Now, the user actively configured a default value to "unknown" and we can introduce new
-	 * behavior without changing old behavior (step 1.).
 	 * Instead of reading static config files in /etc, just read the current sysctl value.
 	 * This works as NM only writes to "/proc/sys/net/ipv6/conf/IFNAME/use_tempaddr", but leaves
 	 * the "default" entry untouched. */