From fab6260bfa4b6452d0d9f1a7ae18d480eac7e61b Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Tue, 19 Nov 2013 19:40:28 -0600 Subject: [PATCH 1/3] policy: ignore nameservers when starting lookup thread (rh #1031763) When generating connections at startup for active interfaces, the generation code may not always be able to read DNS information for the connection. Thus, the device's IP4Config won't have any nameservers and the device won't be considered for reverse-address lookup. However, since any device that gets this far is already the "best" device and has the default route, and thus should be the one used for reverse-address lookup. Second, reorganize the code better handle dual-stack in the future by checking the IP configs directly, instead of the devices. Since 'best4' and 'best6' may be different devices, we want to operate on the IP configs, not devices, to handle situations where the best IP4Config may not be suitable for reverse lookup, but the best IP6Config is. https://bugzilla.redhat.com/show_bug.cgi?id=1031763 --- src/nm-policy.c | 41 +++++++++++++++++++---------------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index fe21d40e51..677f8f9e9e 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -369,6 +369,8 @@ update_system_hostname (NMPolicy *policy, NMDevice *best4, NMDevice *best6) NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (policy); char *configured_hostname = NULL; const char *dhcp_hostname, *p; + NMIP4Config *ip4_config; + NMIP6Config *ip6_config; g_return_if_fail (policy != NULL); @@ -459,42 +461,37 @@ update_system_hostname (NMPolicy *policy, NMDevice *best4, NMDevice *best6) /* No configured hostname, no automatically determined hostname, and no * bootup hostname. Start reverse DNS of the current IPv4 or IPv6 address. */ - if (best4) { - NMIP4Config *ip4_config; - const NMPlatformIP4Address *addr4; + ip4_config = best4 ? nm_device_get_ip4_config (best4) : NULL; + ip6_config = best6 ? nm_device_get_ip6_config (best6) : NULL; - ip4_config = nm_device_get_ip4_config (best4); - if ( !ip4_config - || (nm_ip4_config_get_num_nameservers (ip4_config) == 0) - || (nm_ip4_config_get_num_addresses (ip4_config) == 0)) { - /* No valid IP4 config (!!); fall back to localhost.localdomain */ - _set_hostname (policy, NULL, "no IPv4 config"); - return; - } + if ( (!ip4_config || (nm_ip4_config_get_num_addresses (ip4_config) == 0)) + && (!ip6_config || (nm_ip6_config_get_num_addresses (ip6_config) == 0))) { + /* No valid IP config; fall back to localhost.localdomain */ + _set_hostname (policy, NULL, "no IP config"); + return; + } + + if (ip4_config) { + const NMPlatformIP4Address *addr4; addr4 = nm_ip4_config_get_address (ip4_config, 0); g_assert (addr4); /* checked for > 1 address above */ priv->lookup_addr = g_inet_address_new_from_bytes ((guint8 *) &addr4->address, G_SOCKET_FAMILY_IPV4); - } else { - NMIP6Config *ip6_config; + } else if (ip6_config) { const NMPlatformIP6Address *addr6; - ip6_config = nm_device_get_ip6_config (best6); - if ( !ip6_config - || (nm_ip6_config_get_num_nameservers (ip6_config) == 0) - || (nm_ip6_config_get_num_addresses (ip6_config) == 0)) { - /* No valid IP6 config (!!); fall back to localhost.localdomain */ - _set_hostname (policy, NULL, "no IPv6 config"); - return; - } - addr6 = nm_ip6_config_get_address (ip6_config, 0); g_assert (addr6); /* checked for > 1 address above */ priv->lookup_addr = g_inet_address_new_from_bytes ((guint8 *) &addr6->address, G_SOCKET_FAMILY_IPV6); + } else { + /* Should never get here... */ + g_warn_if_reached (); + _set_hostname (policy, NULL, "no IP config"); + return; } priv->lookup_cancellable = g_cancellable_new (); From 12d96c30f2d8abf66401d053904b2e9549ff8aef Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Tue, 19 Nov 2013 22:28:36 -0600 Subject: [PATCH 2/3] core: capture DNS configuration from resolv.conf when generating connections If the interface who's IP configuration is being captured has the default route, then read DNS servers from resolv.conf into the NMIP[4|6]Config. This allows NetworkManager to repopulate resolv.conf if anything changes. For example, if the system does not define a persistent hostname, then when a device which has generated a connection activates, a hostname lookup will be performed. The results of that lookup may change resolv.conf, and thus NetworkManager must rewrite resolv.conf. Without capturing DNS information at startup when generating connections, an empty resolv.conf would be written. --- .gitignore | 1 + src/NetworkManagerUtils.c | 48 ++++++ src/NetworkManagerUtils.h | 2 + src/devices/nm-device.c | 10 +- src/nm-ip4-config.c | 59 +++++++- src/nm-ip4-config.h | 8 +- src/nm-ip6-config.c | 61 +++++++- src/nm-ip6-config.h | 8 +- src/tests/Makefile.am | 12 +- src/tests/test-resolvconf-capture.c | 226 ++++++++++++++++++++++++++++ 10 files changed, 425 insertions(+), 10 deletions(-) create mode 100644 src/tests/test-resolvconf-capture.c diff --git a/.gitignore b/.gitignore index 63b992550c..50d14854cf 100644 --- a/.gitignore +++ b/.gitignore @@ -184,6 +184,7 @@ valgrind-*.log /src/tests/test-ip6-config /src/tests/test-policy-hosts /src/tests/test-wifi-ap-utils +/src/tests/test-resolvconf-capture /src/dhcp-manager/tests/test-dhcp-dhclient /src/config/tests/test-config diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c index c2cf5e7a50..4d42ad2d9d 100644 --- a/src/NetworkManagerUtils.c +++ b/src/NetworkManagerUtils.c @@ -25,6 +25,7 @@ #include #include #include +#include #include "NetworkManagerUtils.h" #include "nm-utils.h" @@ -611,3 +612,50 @@ nm_utils_new_vlan_name (const char *parent_iface, guint32 vlan_id) return g_strdup_printf ("%s.%d", parent_iface, vlan_id); } +/** + * nm_utils_read_resolv_conf_nameservers(): + * @rc_contents: contents of a resolv.conf; or %NULL to read /etc/resolv.conf + * + * Reads all nameservers out of @rc_contents or /etc/resolv.conf and returns + * them. + * + * Returns: a #GPtrArray of 'char *' elements of each nameserver line from + * @contents or resolv.conf + */ +GPtrArray * +nm_utils_read_resolv_conf_nameservers (const char *rc_contents) +{ + GPtrArray *nameservers = NULL; + char *contents = NULL; + char **lines, **iter; + char *p; + + if (rc_contents) + contents = g_strdup (rc_contents); + else { + if (!g_file_get_contents (_PATH_RESCONF, &contents, NULL, NULL)) + return NULL; + } + + nameservers = g_ptr_array_new_full (3, g_free); + + lines = g_strsplit_set (contents, "\r\n", -1); + for (iter = lines; *iter; iter++) { + if (!g_str_has_prefix (*iter, "nameserver")) + continue; + p = *iter + strlen ("nameserver"); + if (!g_ascii_isspace (*p++)) + continue; + /* Skip intermediate whitespace */ + while (g_ascii_isspace (*p)) + p++; + g_strchomp (p); + + g_ptr_array_add (nameservers, g_strdup (p)); + } + g_strfreev (lines); + g_free (contents); + + return nameservers; +} + diff --git a/src/NetworkManagerUtils.h b/src/NetworkManagerUtils.h index 93ec11134d..a84ba10741 100644 --- a/src/NetworkManagerUtils.h +++ b/src/NetworkManagerUtils.h @@ -84,4 +84,6 @@ void nm_utils_complete_generic (NMConnection *connection, char *nm_utils_new_vlan_name (const char *parent_iface, guint32 vlan_id); +GPtrArray *nm_utils_read_resolv_conf_nameservers (const char *rc_contents); + #endif /* NETWORK_MANAGER_UTILS_H */ diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 832a959304..d92a94b28a 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -357,7 +357,7 @@ static void link_changed_cb (NMPlatform *platform, int ifindex, NMPlatformLink * static void check_carrier (NMDevice *device); static void nm_device_queued_ip_config_change_clear (NMDevice *self); -static void update_ip_config (NMDevice *self, gboolean capture_dhcp); +static void update_ip_config (NMDevice *self, gboolean initial); static void device_ip_changed (NMPlatform *platform, int ifindex, gpointer platform_object, NMPlatformReason reason, gpointer user_data); static void nm_device_slave_notify_enslave (NMDevice *dev, gboolean success); @@ -6503,7 +6503,7 @@ capture_lease_config (NMDevice *device, } static void -update_ip_config (NMDevice *self, gboolean capture_dhcp) +update_ip_config (NMDevice *self, gboolean initial) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); int ifindex; @@ -6515,10 +6515,10 @@ update_ip_config (NMDevice *self, gboolean capture_dhcp) /* IPv4 */ g_clear_object (&priv->ext_ip4_config); - priv->ext_ip4_config = nm_ip4_config_capture (ifindex); + priv->ext_ip4_config = nm_ip4_config_capture (ifindex, initial); if (priv->ext_ip4_config) { - if (capture_dhcp) { + if (initial) { g_clear_object (&priv->dev_ip4_config); capture_lease_config (self, priv->ext_ip4_config, &priv->dev_ip4_config, NULL, NULL); } @@ -6532,7 +6532,7 @@ update_ip_config (NMDevice *self, gboolean capture_dhcp) /* IPv6 */ g_clear_object (&priv->ext_ip6_config); - priv->ext_ip6_config = nm_ip6_config_capture (ifindex); + priv->ext_ip6_config = nm_ip6_config_capture (ifindex, initial); if (priv->ext_ip6_config) { /* Check this before modifying ext_ip6_config */ diff --git a/src/nm-ip4-config.c b/src/nm-ip4-config.c index 5229ef9822..e3b72e2a29 100644 --- a/src/nm-ip4-config.c +++ b/src/nm-ip4-config.c @@ -29,6 +29,7 @@ #include "nm-dbus-manager.h" #include "nm-dbus-glib-types.h" #include "nm-ip4-config-glue.h" +#include "NetworkManagerUtils.h" G_DEFINE_TYPE (NMIP4Config, nm_ip4_config, G_TYPE_OBJECT) @@ -111,6 +112,52 @@ same_prefix (guint32 address1, guint32 address2, int plen) /******************************************************************/ +/** + * nm_ip4_config_capture_resolv_conf(): + * @nameservers: array of guint32 + * @rc_contents: the contents of a resolv.conf or %NULL to read /etc/resolv.conf + * + * Reads all resolv.conf IPv6 nameservers and adds them to @nameservers. + * + * Returns: %TRUE if nameservers were added, %FALSE if @nameservers is unchanged + */ +gboolean +nm_ip4_config_capture_resolv_conf (GArray *nameservers, + const char *rc_contents) +{ + GPtrArray *read_ns; + guint i, j; + gboolean changed = FALSE; + + g_return_val_if_fail (nameservers != NULL, FALSE); + + read_ns = nm_utils_read_resolv_conf_nameservers (rc_contents); + if (!read_ns) + return FALSE; + + for (i = 0; i < read_ns->len; i++) { + const char *s = g_ptr_array_index (read_ns, i); + guint32 ns = 0; + + if (!inet_pton (AF_INET, s, (void *) &ns) || !ns) + continue; + + /* Ignore duplicates */ + for (j = 0; j < nameservers->len; j++) { + if (g_array_index (nameservers, guint32, j) == ns) + break; + } + + if (j == nameservers->len) { + g_array_append_val (nameservers, ns); + changed = TRUE; + } + } + + g_ptr_array_unref (read_ns); + return changed; +} + static gboolean addresses_are_duplicate (const NMPlatformIP4Address *a, const NMPlatformIP4Address *b, gboolean consider_plen) { @@ -125,12 +172,13 @@ routes_are_duplicate (const NMPlatformIP4Route *a, const NMPlatformIP4Route *b, } NMIP4Config * -nm_ip4_config_capture (int ifindex) +nm_ip4_config_capture (int ifindex, gboolean capture_resolv_conf) { NMIP4Config *config; NMIP4ConfigPrivate *priv; guint i; gboolean gateway_changed = FALSE; + gboolean has_gateway = FALSE; /* Slaves have no IP configuration */ if (nm_platform_link_get_master (ifindex) > 0) @@ -154,12 +202,21 @@ nm_ip4_config_capture (int ifindex) priv->gateway = route->gateway; gateway_changed = TRUE; } + has_gateway = TRUE; /* Remove the default route from the list */ g_array_remove_index (priv->routes, i); break; } } + /* If the interface has the default route, and has IPv4 addresses, capture + * nameservers from /etc/resolv.conf. + */ + if (priv->addresses->len && has_gateway && capture_resolv_conf) { + if (nm_ip4_config_capture_resolv_conf (priv->nameservers, NULL)) + _NOTIFY (config, PROP_NAMESERVERS); + } + /* actually, nobody should be connected to the signal, just to be sure, notify */ _NOTIFY (config, PROP_ADDRESSES); _NOTIFY (config, PROP_ROUTES); diff --git a/src/nm-ip4-config.h b/src/nm-ip4-config.h index 3b2b250c93..5b76fb4120 100644 --- a/src/nm-ip4-config.h +++ b/src/nm-ip4-config.h @@ -59,7 +59,7 @@ void nm_ip4_config_export (NMIP4Config *config); const char * nm_ip4_config_get_dbus_path (const NMIP4Config *config); /* Integration with nm-platform and nm-setting */ -NMIP4Config *nm_ip4_config_capture (int ifindex); +NMIP4Config *nm_ip4_config_capture (int ifindex, gboolean capture_resolv_conf); gboolean nm_ip4_config_commit (const NMIP4Config *config, int ifindex, int priority); void nm_ip4_config_merge_setting (NMIP4Config *config, NMSettingIP4Config *setting); void nm_ip4_config_update_setting (const NMIP4Config *config, NMSettingIP4Config *setting); @@ -144,4 +144,10 @@ guint32 nm_ip4_config_get_mtu (const NMIP4Config *config); void nm_ip4_config_hash (const NMIP4Config *config, GChecksum *sum, gboolean dns_only); gboolean nm_ip4_config_equal (const NMIP4Config *a, const NMIP4Config *b); +/******************************************************/ +/* Testing-only functions */ + +gboolean nm_ip4_config_capture_resolv_conf (GArray *nameservers, + const char *rc_contents); + #endif /* NM_IP4_CONFIG_H */ diff --git a/src/nm-ip6-config.c b/src/nm-ip6-config.c index 7522164b37..6b887aa8bf 100644 --- a/src/nm-ip6-config.c +++ b/src/nm-ip6-config.c @@ -30,6 +30,7 @@ #include "nm-dbus-manager.h" #include "nm-dbus-glib-types.h" #include "nm-ip6-config-glue.h" +#include "NetworkManagerUtils.h" G_DEFINE_TYPE (NMIP6Config, nm_ip6_config, G_TYPE_OBJECT) @@ -111,6 +112,54 @@ same_prefix (const struct in6_addr *address1, const struct in6_addr *address2, i /******************************************************************/ +/** + * nm_ip6_config_capture_resolv_conf(): + * @nameservers: array of struct in6_addr + * @rc_contents: the contents of a resolv.conf or %NULL to read /etc/resolv.conf + * + * Reads all resolv.conf IPv6 nameservers and adds them to @nameservers. + * + * Returns: %TRUE if nameservers were added, %FALSE if @nameservers is unchanged + */ +gboolean +nm_ip6_config_capture_resolv_conf (GArray *nameservers, + const char *rc_contents) +{ + GPtrArray *read_ns; + guint i, j; + gboolean changed = FALSE; + + g_return_val_if_fail (nameservers != NULL, FALSE); + + read_ns = nm_utils_read_resolv_conf_nameservers (rc_contents); + if (!read_ns) + return FALSE; + + for (i = 0; i < read_ns->len; i++) { + const char *s = g_ptr_array_index (read_ns, i); + struct in6_addr ns = IN6ADDR_ANY_INIT; + + if (!inet_pton (AF_INET6, s, (void *) &ns) || IN6_IS_ADDR_UNSPECIFIED (&ns)) + continue; + + /* Ignore duplicates */ + for (j = 0; j < nameservers->len; j++) { + struct in6_addr *t = &g_array_index (nameservers, struct in6_addr, j); + + if (IN6_ARE_ADDR_EQUAL (t, &ns)) + break; + } + + if (j == nameservers->len) { + g_array_append_val (nameservers, ns); + changed = TRUE; + } + } + + g_ptr_array_unref (read_ns); + return changed; +} + static gboolean addresses_are_duplicate (const NMPlatformIP6Address *a, const NMPlatformIP6Address *b, gboolean consider_plen) { @@ -125,12 +174,13 @@ routes_are_duplicate (const NMPlatformIP6Route *a, const NMPlatformIP6Route *b, } NMIP6Config * -nm_ip6_config_capture (int ifindex) +nm_ip6_config_capture (int ifindex, gboolean capture_resolv_conf) { NMIP6Config *config; NMIP6ConfigPrivate *priv; guint i; gboolean gateway_changed = FALSE; + gboolean has_gateway = FALSE; /* Slaves have no IP configuration */ if (nm_platform_link_get_master (ifindex) > 0) @@ -154,12 +204,21 @@ nm_ip6_config_capture (int ifindex) priv->gateway = route->gateway; gateway_changed = TRUE; } + has_gateway = TRUE; /* Remove the default route from the list */ g_array_remove_index (priv->routes, i); break; } } + /* If the interface has the default route, and has IPv4 addresses, capture + * nameservers from /etc/resolv.conf. + */ + if (priv->addresses->len && has_gateway && capture_resolv_conf) { + if (nm_ip6_config_capture_resolv_conf (priv->nameservers, NULL)) + _NOTIFY (config, PROP_NAMESERVERS); + } + /* actually, nobody should be connected to the signal, just to be sure, notify */ _NOTIFY (config, PROP_ADDRESSES); _NOTIFY (config, PROP_ROUTES); diff --git a/src/nm-ip6-config.h b/src/nm-ip6-config.h index 538490a78c..aecdab2a81 100644 --- a/src/nm-ip6-config.h +++ b/src/nm-ip6-config.h @@ -58,7 +58,7 @@ void nm_ip6_config_export (NMIP6Config *config); const char * nm_ip6_config_get_dbus_path (const NMIP6Config *config); /* Integration with nm-platform and nm-setting */ -NMIP6Config *nm_ip6_config_capture (int ifindex); +NMIP6Config *nm_ip6_config_capture (int ifindex, gboolean capture_resolv_conf); gboolean nm_ip6_config_commit (const NMIP6Config *config, int ifindex, int priority); void nm_ip6_config_merge_setting (NMIP6Config *config, NMSettingIP6Config *setting); void nm_ip6_config_update_setting (const NMIP6Config *config, NMSettingIP6Config *setting); @@ -123,4 +123,10 @@ const struct in6_addr *nm_ip6_config_get_ptp_address (const NMIP6Config *config) void nm_ip6_config_hash (const NMIP6Config *config, GChecksum *sum, gboolean dns_only); gboolean nm_ip6_config_equal (const NMIP6Config *a, const NMIP6Config *b); +/******************************************************/ +/* Testing-only functions */ + +gboolean nm_ip6_config_capture_resolv_conf (GArray *nameservers, + const char *rc_contents); + #endif /* NM_IP6_CONFIG_H */ diff --git a/src/tests/Makefile.am b/src/tests/Makefile.am index 9d17e9ad1a..eaa9d516fc 100644 --- a/src/tests/Makefile.am +++ b/src/tests/Makefile.am @@ -16,7 +16,8 @@ noinst_PROGRAMS = \ test-wifi-ap-utils \ test-ip4-config \ test-ip6-config \ - test-dcb + test-dcb \ + test-resolvconf-capture ####### DHCP options test ####### @@ -71,6 +72,14 @@ test_dcb_SOURCES = \ test_dcb_LDADD = \ $(top_builddir)/src/libNetworkManager.la +####### resolv.conf capture test ####### + +test_resolvconf_capture_SOURCES = \ + test-resolvconf-capture.c + +test_resolvconf_capture_LDADD = \ + $(top_builddir)/src/libNetworkManager.la + ####### secret agent interface test ####### EXTRA_DIST = test-secret-agent.py @@ -84,4 +93,5 @@ check-local: test-dhcp-options test-policy-hosts test-wifi-ap-utils test-ip4-con $(abs_builddir)/test-ip4-config $(abs_builddir)/test-ip6-config $(abs_builddir)/test-dcb + $(abs_builddir)/test-resolvconf-capture diff --git a/src/tests/test-resolvconf-capture.c b/src/tests/test-resolvconf-capture.c new file mode 100644 index 0000000000..a04cbae8f2 --- /dev/null +++ b/src/tests/test-resolvconf-capture.c @@ -0,0 +1,226 @@ +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */ +/* + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Copyright (C) 2013 Red Hat, Inc. + * + */ + +#include +#include + +#include "NetworkManagerUtils.h" +#include "nm-platform.h" + +static void +test_capture_empty (void) +{ + GArray *ns4 = g_array_new (FALSE, FALSE, sizeof (guint32)); + GArray *ns6 = g_array_new (FALSE, FALSE, sizeof (struct in6_addr)); + + g_assert (nm_ip4_config_capture_resolv_conf (ns4, "") == FALSE); + g_assert_cmpint (ns4->len, ==, 0); + + g_assert (nm_ip6_config_capture_resolv_conf (ns6, "") == FALSE); + g_assert_cmpint (ns6->len, ==, 0); + + g_array_free (ns4, TRUE); + g_array_free (ns6, TRUE); +} + +static void +assert_dns4_entry (const GArray *a, guint i, const char *s) +{ + guint32 n, m; + + g_assert (inet_aton (s, (void *) &n) != 0); + m = g_array_index (a, guint32, i); + g_assert_cmpint (m, ==, n); +} + +static void +assert_dns6_entry (const GArray *a, guint i, const char *s) +{ + struct in6_addr n = IN6ADDR_ANY_INIT; + struct in6_addr *m; + + g_assert (inet_pton (AF_INET6, s, (void *) &n) == 1); + m = &g_array_index (a, struct in6_addr, i); + g_assert (IN6_ARE_ADDR_EQUAL (&n, m)); +} + +static void +test_capture_basic4 (void) +{ + GArray *ns4 = g_array_new (FALSE, FALSE, sizeof (guint32)); + const char *rc = +"# neato resolv.conf\r\n" +"domain foobar.com\r\n" +"search foobar.com\r\n" +"nameserver 4.2.2.1\r\n" +"nameserver 4.2.2.2\r\n"; + + g_assert (nm_ip4_config_capture_resolv_conf (ns4, rc)); + g_assert_cmpint (ns4->len, ==, 2); + assert_dns4_entry (ns4, 0, "4.2.2.1"); + assert_dns4_entry (ns4, 1, "4.2.2.2"); + + g_array_free (ns4, TRUE); +} + +static void +test_capture_dup4 (void) +{ + GArray *ns4 = g_array_new (FALSE, FALSE, sizeof (guint32)); + const char *rc = +"# neato resolv.conf\r\n" +"domain foobar.com\r\n" +"search foobar.com\r\n" +"nameserver 4.2.2.1\r\n" +"nameserver 4.2.2.1\r\n" +"nameserver 4.2.2.2\r\n"; + + /* Check that duplicates are ignored */ + g_assert (nm_ip4_config_capture_resolv_conf (ns4, rc)); + g_assert_cmpint (ns4->len, ==, 2); + assert_dns4_entry (ns4, 0, "4.2.2.1"); + assert_dns4_entry (ns4, 1, "4.2.2.2"); + + g_array_free (ns4, TRUE); +} + +static void +test_capture_basic6 (void) +{ + GArray *ns6 = g_array_new (FALSE, FALSE, sizeof (struct in6_addr)); + const char *rc = +"# neato resolv.conf\r\n" +"domain foobar.com\r\n" +"search foobar.com\r\n" +"nameserver 2001:4860:4860::8888\r\n" +"nameserver 2001:4860:4860::8844\r\n"; + + g_assert (nm_ip6_config_capture_resolv_conf (ns6, rc)); + g_assert_cmpint (ns6->len, ==, 2); + assert_dns6_entry (ns6, 0, "2001:4860:4860::8888"); + assert_dns6_entry (ns6, 1, "2001:4860:4860::8844"); + + g_array_free (ns6, TRUE); +} + +static void +test_capture_dup6 (void) +{ + GArray *ns6 = g_array_new (FALSE, FALSE, sizeof (struct in6_addr)); + const char *rc = +"# neato resolv.conf\r\n" +"domain foobar.com\r\n" +"search foobar.com\r\n" +"nameserver 2001:4860:4860::8888\r\n" +"nameserver 2001:4860:4860::8888\r\n" +"nameserver 2001:4860:4860::8844\r\n"; + + /* Check that duplicates are ignored */ + g_assert (nm_ip6_config_capture_resolv_conf (ns6, rc)); + g_assert_cmpint (ns6->len, ==, 2); + assert_dns6_entry (ns6, 0, "2001:4860:4860::8888"); + assert_dns6_entry (ns6, 1, "2001:4860:4860::8844"); + + g_array_free (ns6, TRUE); +} + +static void +test_capture_addr4_with_6 (void) +{ + GArray *ns4 = g_array_new (FALSE, FALSE, sizeof (guint32)); + const char *rc = +"# neato resolv.conf\r\n" +"domain foobar.com\r\n" +"search foobar.com\r\n" +"nameserver 4.2.2.1\r\n" +"nameserver 4.2.2.2\r\n" +"nameserver 2001:4860:4860::8888\r\n"; + + g_assert (nm_ip4_config_capture_resolv_conf (ns4, rc)); + g_assert_cmpint (ns4->len, ==, 2); + assert_dns4_entry (ns4, 0, "4.2.2.1"); + assert_dns4_entry (ns4, 1, "4.2.2.2"); + + g_array_free (ns4, TRUE); +} + +static void +test_capture_addr6_with_4 (void) +{ + GArray *ns6 = g_array_new (FALSE, FALSE, sizeof (struct in6_addr)); + const char *rc = +"# neato resolv.conf\r\n" +"domain foobar.com\r\n" +"search foobar.com\r\n" +"nameserver 4.2.2.1\r\n" +"nameserver 2001:4860:4860::8888\r\n" +"nameserver 2001:4860:4860::8844\r\n"; + + g_assert (nm_ip6_config_capture_resolv_conf (ns6, rc)); + g_assert_cmpint (ns6->len, ==, 2); + assert_dns6_entry (ns6, 0, "2001:4860:4860::8888"); + assert_dns6_entry (ns6, 1, "2001:4860:4860::8844"); + + g_array_free (ns6, TRUE); +} + +static void +test_capture_format (void) +{ + GArray *ns4 = g_array_new (FALSE, FALSE, sizeof (guint32)); + const char *rc = +" nameserver 4.2.2.1\r\n" /* bad */ +"nameserver4.2.2.1\r\n" /* bad */ +"nameserver 4.2.2.3\r" /* good */ +"nameserver\t\t4.2.2.4\r\n" /* good */ +"nameserver 4.2.2.5\t\t\r\n"; /* good */ + + g_assert (nm_ip4_config_capture_resolv_conf (ns4, rc)); + g_assert_cmpint (ns4->len, ==, 3); + assert_dns4_entry (ns4, 0, "4.2.2.3"); + assert_dns4_entry (ns4, 1, "4.2.2.4"); + assert_dns4_entry (ns4, 2, "4.2.2.5"); + + g_array_free (ns4, TRUE); +} + +/*******************************************/ + +int +main (int argc, char **argv) +{ + g_test_init (&argc, &argv, NULL); + +#if !GLIB_CHECK_VERSION (2,36,0) + g_type_init (); +#endif + + g_test_add_func ("/resolvconf-capture/empty", test_capture_empty); + g_test_add_func ("/resolvconf-capture/basic4", test_capture_basic4); + g_test_add_func ("/resolvconf-capture/dup4", test_capture_dup4); + g_test_add_func ("/resolvconf-capture/basic6", test_capture_basic6); + g_test_add_func ("/resolvconf-capture/dup6", test_capture_dup6); + g_test_add_func ("/resolvconf-capture/addr4-with-6", test_capture_addr4_with_6); + g_test_add_func ("/resolvconf-capture/addr6-with-4", test_capture_addr6_with_4); + g_test_add_func ("/resolvconf-capture/format", test_capture_format); + + return g_test_run (); +} + From da016d91f5e43c7e0217362e4df4d2850f74f820 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Tue, 19 Nov 2013 22:51:29 -0600 Subject: [PATCH 3/3] core: don't leave additional default routes in captured IP config There can be multiple default routes for an interface with different metrics. Grab the gateway of the default route with the lowest metric as the overall gateway of the IP config. Otherwise the rest could get left in the config and applied at random times. --- src/nm-ip4-config.c | 12 +++++++----- src/nm-ip6-config.c | 13 ++++++++----- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/nm-ip4-config.c b/src/nm-ip4-config.c index e3b72e2a29..7aff888c0d 100644 --- a/src/nm-ip4-config.c +++ b/src/nm-ip4-config.c @@ -177,7 +177,8 @@ nm_ip4_config_capture (int ifindex, gboolean capture_resolv_conf) NMIP4Config *config; NMIP4ConfigPrivate *priv; guint i; - gboolean gateway_changed = FALSE; + guint lowest_metric = G_MAXUINT; + guint32 old_gateway = 0; gboolean has_gateway = FALSE; /* Slaves have no IP configuration */ @@ -194,18 +195,19 @@ nm_ip4_config_capture (int ifindex, gboolean capture_resolv_conf) priv->routes = nm_platform_ip4_route_get_all (ifindex, TRUE); /* Extract gateway from default route */ + old_gateway = priv->gateway; for (i = 0; i < priv->routes->len; i++) { const NMPlatformIP4Route *route = &g_array_index (priv->routes, NMPlatformIP4Route, i); if (route->network == 0) { - if (priv->gateway != route->gateway) { + if (route->metric < lowest_metric) { priv->gateway = route->gateway; - gateway_changed = TRUE; + lowest_metric = route->metric; } has_gateway = TRUE; /* Remove the default route from the list */ g_array_remove_index (priv->routes, i); - break; + i--; } } @@ -220,7 +222,7 @@ nm_ip4_config_capture (int ifindex, gboolean capture_resolv_conf) /* actually, nobody should be connected to the signal, just to be sure, notify */ _NOTIFY (config, PROP_ADDRESSES); _NOTIFY (config, PROP_ROUTES); - if (gateway_changed) + if (priv->gateway != old_gateway) _NOTIFY (config, PROP_GATEWAY); return config; diff --git a/src/nm-ip6-config.c b/src/nm-ip6-config.c index 6b887aa8bf..65cbd0be54 100644 --- a/src/nm-ip6-config.c +++ b/src/nm-ip6-config.c @@ -179,7 +179,8 @@ nm_ip6_config_capture (int ifindex, gboolean capture_resolv_conf) NMIP6Config *config; NMIP6ConfigPrivate *priv; guint i; - gboolean gateway_changed = FALSE; + guint lowest_metric = G_MAXUINT; + struct in6_addr old_gateway = IN6ADDR_ANY_INIT; gboolean has_gateway = FALSE; /* Slaves have no IP configuration */ @@ -196,18 +197,20 @@ nm_ip6_config_capture (int ifindex, gboolean capture_resolv_conf) priv->routes = nm_platform_ip6_route_get_all (ifindex, TRUE); /* Extract gateway from default route */ + old_gateway = priv->gateway; for (i = 0; i < priv->routes->len; i++) { const NMPlatformIP6Route *route = &g_array_index (priv->routes, NMPlatformIP6Route, i); if (IN6_IS_ADDR_UNSPECIFIED (&route->network)) { - if (!IN6_ARE_ADDR_EQUAL (&priv->gateway, &route->gateway)) { + if (route->metric < lowest_metric) { priv->gateway = route->gateway; - gateway_changed = TRUE; + lowest_metric = route->metric; + has_gateway = TRUE; } has_gateway = TRUE; /* Remove the default route from the list */ g_array_remove_index (priv->routes, i); - break; + i--; } } @@ -222,7 +225,7 @@ nm_ip6_config_capture (int ifindex, gboolean capture_resolv_conf) /* actually, nobody should be connected to the signal, just to be sure, notify */ _NOTIFY (config, PROP_ADDRESSES); _NOTIFY (config, PROP_ROUTES); - if (gateway_changed) + if (!IN6_ARE_ADDR_EQUAL (&priv->gateway, &old_gateway)) _NOTIFY (config, PROP_GATEWAY); return config;