From 2ca21e150a01a210028b5eae03bd1611af7b37bd Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 3 Jun 2016 10:27:07 +0200 Subject: [PATCH 1/5] configure/trivial: prettify "if" in configure.ac --- configure.ac | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index dbcc024572..cad6b4b6f4 100644 --- a/configure.ac +++ b/configure.ac @@ -780,7 +780,10 @@ fi AC_ARG_WITH(resolvconf, AS_HELP_STRING([--with-resolvconf=yes|no|path], [Enable resolvconf support])) AC_ARG_WITH(netconfig, AS_HELP_STRING([--with-netconfig=yes|no], [Enable SUSE netconfig support])) AC_ARG_WITH(config-dns-rc-manager-default, AS_HELP_STRING([--with-config-dns-rc-manager-default=symlink|file|netconfig|resolvconf], [Configure default value for main.rc-manager setting]), [config_dns_rc_manager_default=$withval]) -if test "$config_dns_rc_manager_default" != symlink -a "$config_dns_rc_manager_default" != file -a "$config_dns_rc_manager_default" != netconfig -a "$config_dns_rc_manager_default" != resolvconf; then +if test "$config_dns_rc_manager_default" != symlink -a \ + "$config_dns_rc_manager_default" != file -a \ + "$config_dns_rc_manager_default" != netconfig -a \ + "$config_dns_rc_manager_default" != resolvconf; then AC_MSG_WARN([Unknown --with-config-dns-rc-manager-default=$config_dns_rc_manager_default setting.]) config_dns_rc_manager_default= fi From 9418f815280a52c1e7baf7a1a646da4e8747c044 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 3 Jun 2016 10:28:30 +0200 Subject: [PATCH 2/5] dns: refactor logging statements to use _rc_manager_to_string() Reuse _rc_manager_to_string() to stringify the rc-manager mode. Also fix typo "rc-managed=file". --- src/dns-manager/nm-dns-manager.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/dns-manager/nm-dns-manager.c b/src/dns-manager/nm-dns-manager.c index 114b33d6c0..dfeece6240 100644 --- a/src/dns-manager/nm-dns-manager.c +++ b/src/dns-manager/nm-dns-manager.c @@ -701,14 +701,14 @@ update_resolv_conf (NMDnsManager *self, * we still continue to write to runstatedir but remember the * error. */ if (!g_file_set_contents (_PATH_RESCONF, content, -1, &local)) { - _LOGT ("update-resolv-conf: write to %s failed (rc-managed=file, %s)", - _PATH_RESCONF, local->message); + _LOGT ("update-resolv-conf: write to %s failed (rc-manager=%s, %s)", + _PATH_RESCONF, _rc_manager_to_string (rc_manager), local->message); write_file_result = SR_ERROR; g_propagate_error (error, local); error = NULL; } else { - _LOGT ("update-resolv-conf: write to %s succeeded (rc-managed=file)", - _PATH_RESCONF); + _LOGT ("update-resolv-conf: write to %s succeeded (rc-manager=%s)", + _PATH_RESCONF, _rc_manager_to_string (rc_manager)); } } @@ -765,8 +765,8 @@ update_resolv_conf (NMDnsManager *self, } if (rc_manager == NM_DNS_MANAGER_RESOLV_CONF_MAN_FILE) { - _LOGT ("update-resolv-conf: write internal file %s succeeded (rc-manager=file)", - MY_RESOLV_CONF); + _LOGT ("update-resolv-conf: write internal file %s succeeded (rc-manager=%s)", + _PATH_RESCONF, _rc_manager_to_string (rc_manager)); return write_file_result; } From 718fd2243690b8c72dd1cb32f67114f304542082 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 3 Jun 2016 10:33:54 +0200 Subject: [PATCH 3/5] dns: follow resolv.conf if it is a symlink for 'rc-manager=file' Until before 1.2.0, NetworkManager would always write resolv.conf as file, but if /etc/resolv.conf was a symlink, it would follow the link instead of replacing it with a file ([1], [2]). With 1.2.0, we initially dropped that behavior and added a new 'rc-manager=none' which writes resolv.conf to /var/run/NetworkManager and symlinks resolv.conf [3]. In case resolv.conf being already a symlink to another target, it would not be replaced [4]. Later, we added 'rc-manager=file', which always writes /etc/resolv.conf as file [5]. With 1.4.0, we will rename 'rc-manager=none' to 'rc-manager=symlink' [6]. This commit now fixes 'rc-manager=file' to restores the pre-1.2 behavior and follow symlinks. [1] 5761e328b81ce8894c2657ce0994ba401923ba35 [2] https://bugs.launchpad.net/ubuntu/+source/network-manager/+bug/324233 [3] 4805be2ed27b71a6099477d86dbc109adb41b819 [4] 583568e12f9e580cd2903811637c9f9b7a2f1088 [5] 288799713dc78bc45e2b0a9cf41d228f5d95315f [6] cd6a469668028fbc347919ed3580275f9894a1f2 https://github.com/NetworkManager/NetworkManager/pull/7 --- man/NetworkManager.conf.xml | 4 +++- src/dns-manager/nm-dns-manager.c | 14 ++++++++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/man/NetworkManager.conf.xml b/man/NetworkManager.conf.xml index 23038dbbc1..a6614e3393 100644 --- a/man/NetworkManager.conf.xml +++ b/man/NetworkManager.conf.xml @@ -329,7 +329,9 @@ no-auto-default=* by pointing the link /etc/resolv.conf to somewhere else. file: NetworkManager will write - /etc/resolv.conf as file. + /etc/resolv.conf as file. If it finds + a symlink, it will follow the symlink and update the target + instead. resolvconf: NetworkManager will run resolvconf to update the DNS configuration. netconfig: NetworkManager will run diff --git a/src/dns-manager/nm-dns-manager.c b/src/dns-manager/nm-dns-manager.c index dfeece6240..8134f2f4a4 100644 --- a/src/dns-manager/nm-dns-manager.c +++ b/src/dns-manager/nm-dns-manager.c @@ -674,6 +674,8 @@ update_resolv_conf (NMDnsManager *self, gs_free char *content = NULL; SpawnResult write_file_result = SR_SUCCESS; int errsv; + const char *rc_path = _PATH_RESCONF; + nm_auto_free char *rc_path_real = NULL; /* If we are not managing /etc/resolv.conf and it points to * MY_RESOLV_CONF, don't write the private DNS configuration to @@ -697,18 +699,22 @@ update_resolv_conf (NMDnsManager *self, if (rc_manager == NM_DNS_MANAGER_RESOLV_CONF_MAN_FILE) { GError *local = NULL; + rc_path_real = realpath (rc_path, NULL); + if (rc_path_real) + rc_path = rc_path_real; + /* we first write to /etc/resolv.conf directly. If that fails, * we still continue to write to runstatedir but remember the * error. */ - if (!g_file_set_contents (_PATH_RESCONF, content, -1, &local)) { + if (!g_file_set_contents (rc_path, content, -1, &local)) { _LOGT ("update-resolv-conf: write to %s failed (rc-manager=%s, %s)", - _PATH_RESCONF, _rc_manager_to_string (rc_manager), local->message); + rc_path, _rc_manager_to_string (rc_manager), local->message); write_file_result = SR_ERROR; g_propagate_error (error, local); error = NULL; } else { _LOGT ("update-resolv-conf: write to %s succeeded (rc-manager=%s)", - _PATH_RESCONF, _rc_manager_to_string (rc_manager)); + rc_path, _rc_manager_to_string (rc_manager)); } } @@ -766,7 +772,7 @@ update_resolv_conf (NMDnsManager *self, if (rc_manager == NM_DNS_MANAGER_RESOLV_CONF_MAN_FILE) { _LOGT ("update-resolv-conf: write internal file %s succeeded (rc-manager=%s)", - _PATH_RESCONF, _rc_manager_to_string (rc_manager)); + rc_path, _rc_manager_to_string (rc_manager)); return write_file_result; } From 47118679151ac1296ed9b8e2b3edcf90a38004fe Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 3 Jun 2016 11:16:03 +0200 Subject: [PATCH 4/5] dns: update detection of immutable resolv.conf Before, we would first check whether the file is immuable before parsing main.rc-manager setting. That means, if you configured [main] dns=default rc-manager=unmanged we would still first try to detect whether the file is immutable. The result of course is only minor, e.g. showing up in logging as rc-manager=immutable instead of rc-manager=unmanged. Also, an immutable resolv.conf would suppress a warning about a bogus rc-manager setting. Also, when selecting rc-manager=symlink and resolv.conf is a symlink to an immutable file, we don't actually care about that. The reason is, that if the link-target is not /var/run/NetworkManager/resolv.conf, we anyway wouldn't modify the file. The effect of this change is pretty minor, now in logging you would see: dns-mgr: init: dns=default, rc-manager=symlink dns-mgr: update-resolv-conf: write internal file /var/run/NetworkManager/resolv.conf succeeded but don't update /etc/resolv.conf as it points to /some/where/else instead of dns-mgr: init: dns=default, rc-manager=immutable dns-mgr: update-resolv-conf: write internal file /var/run/NetworkManager/resolv.conf succeeded Which feels slightly more right. Note that symlinks cannot have file attributes. --- src/dns-manager/nm-dns-manager.c | 54 ++++++++++++++++++++++++++------ 1 file changed, 44 insertions(+), 10 deletions(-) diff --git a/src/dns-manager/nm-dns-manager.c b/src/dns-manager/nm-dns-manager.c index 8134f2f4a4..8690be8a36 100644 --- a/src/dns-manager/nm-dns-manager.c +++ b/src/dns-manager/nm-dns-manager.c @@ -1508,19 +1508,53 @@ _clear_plugin (NMDnsManager *self) return FALSE; } -static bool -_get_resconf_immutable (void) +static NMDnsManagerResolvConfManager +_check_resconf_immutable (NMDnsManagerResolvConfManager rc_manager) { + struct stat st; int fd, flags; bool immutable = FALSE; - fd = open (_PATH_RESCONF, O_RDONLY); - if (fd != -1) { - if (ioctl (fd, FS_IOC_GETFLAGS, &flags) != -1) - immutable = NM_FLAGS_HAS (flags, FS_IMMUTABLE_FL); - close (fd); + switch (rc_manager) { + case NM_DNS_MANAGER_RESOLV_CONF_MAN_UNKNOWN: + case NM_DNS_MANAGER_RESOLV_CONF_MAN_IMMUTABLE: + nm_assert_not_reached (); + /* fall-through */ + case NM_DNS_MANAGER_RESOLV_CONF_MAN_UNMANAGED: + return NM_DNS_MANAGER_RESOLV_CONF_MAN_UNMANAGED; + default: + + if (lstat (_PATH_RESCONF, &st) != 0) + return rc_manager; + + if (S_ISLNK (st.st_mode)) { + /* only regular files and directories can have extended file attributes. */ + switch (rc_manager) { + case NM_DNS_MANAGER_RESOLV_CONF_MAN_SYMLINK: + /* we don't care whether the link-target is immutable. + * If the symlink points to another file, rc-manager=symlink anyway backs off. + * Otherwise, we would only check whether our internal resolv.conf is immutable. */ + return NM_DNS_MANAGER_RESOLV_CONF_MAN_SYMLINK; + case NM_DNS_MANAGER_RESOLV_CONF_MAN_UNKNOWN: + case NM_DNS_MANAGER_RESOLV_CONF_MAN_UNMANAGED: + case NM_DNS_MANAGER_RESOLV_CONF_MAN_IMMUTABLE: + nm_assert_not_reached (); + /* fall-through */ + case NM_DNS_MANAGER_RESOLV_CONF_MAN_FILE: + case NM_DNS_MANAGER_RESOLV_CONF_MAN_RESOLVCONF: + case NM_DNS_MANAGER_RESOLV_CONF_MAN_NETCONFIG: + break; + } + } + + fd = open (_PATH_RESCONF, O_RDONLY); + if (fd != -1) { + if (ioctl (fd, FS_IOC_GETFLAGS, &flags) != -1) + immutable = NM_FLAGS_HAS (flags, FS_IMMUTABLE_FL); + close (fd); + } + return immutable ? NM_DNS_MANAGER_RESOLV_CONF_MAN_IMMUTABLE : rc_manager; } - return immutable; } NM_DEFINE_SINGLETON_GETTER (NMDnsManager, nm_dns_manager_get, NM_TYPE_DNS_MANAGER); @@ -1537,8 +1571,6 @@ init_resolv_conf_mode (NMDnsManager *self, gboolean force_reload_plugin) if (nm_streq0 (mode, "none")) rc_manager = NM_DNS_MANAGER_RESOLV_CONF_MAN_UNMANAGED; - else if (_get_resconf_immutable ()) - rc_manager = NM_DNS_MANAGER_RESOLV_CONF_MAN_IMMUTABLE; else { const char *man; @@ -1570,6 +1602,8 @@ again: } } + rc_manager = _check_resconf_immutable (rc_manager); + if (nm_streq0 (mode, "dnsmasq")) { if (force_reload_plugin || !NM_IS_DNS_DNSMASQ (priv->plugin)) { _clear_plugin (self); From bcb88d540e1d555e35f84e6865e1c6adb6992290 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 3 Jun 2016 11:56:59 +0200 Subject: [PATCH 5/5] dns: minor fix of logging with unset dns mode With [main] #dns= we would see in the log: dns-mgr: init: dns=(null), rc-manager=symlink Instead, it should be dns-mgr: init: dns=default, rc-manager=symlink Also, we should avoid logging NULL values with "%s", although glib's printf is fine with that. --- src/dns-manager/nm-dns-manager.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/dns-manager/nm-dns-manager.c b/src/dns-manager/nm-dns-manager.c index 8690be8a36..6c29bd390f 100644 --- a/src/dns-manager/nm-dns-manager.c +++ b/src/dns-manager/nm-dns-manager.c @@ -1617,8 +1617,9 @@ again: plugin_changed = TRUE; } } else { - if (!NM_IN_STRSET (mode, NULL, "none", "default")) { - _LOGW ("init: unknown dns mode '%s'", mode); + if (!NM_IN_STRSET (mode, "none", "default")) { + if (mode) + _LOGW ("init: unknown dns mode '%s'", mode); mode = "default"; } if (_clear_plugin (self))