From 5f1a62877ce7c7179e1954d0ea939123b4ff6713 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 19 Dec 2013 10:58:46 -0600 Subject: [PATCH 1/8] rdisc: clear domain list when updating from RA Appears to be a simple oversight. If the IPv6 router changes the domain list, old domains could stick around. --- src/devices/nm-device.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 74d443d3ab..bdeffc4869 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -3319,6 +3319,9 @@ rdisc_config_changed (NMRDisc *rdisc, NMRDiscConfigMap changed, NMDevice *device } if (changed & NM_RDISC_CONFIG_DNS_DOMAINS) { + /* Rebuild domain list from router discovery cache. */ + nm_ip6_config_reset_domains (priv->ac_ip6_config); + for (i = 0; i < rdisc->dns_domains->len; i++) { NMRDiscDNSDomain *discovered_domain = &g_array_index (rdisc->dns_domains, NMRDiscDNSDomain, i); From 5ef3d9027d10549113c1aaf85a4238745722e582 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 19 Dec 2013 11:00:54 -0600 Subject: [PATCH 2/8] trivial: show interface name in IPv6 rdisc log message --- src/devices/nm-device.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index bdeffc4869..6f2383b753 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -3364,6 +3364,7 @@ addrconf6_start (NMDevice *self) NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); NMConnection *connection; NMActStageReturn ret; + const char *ip_iface = nm_device_get_ip_iface (self); connection = nm_device_get_connection (self); g_assert (connection); @@ -3374,15 +3375,14 @@ addrconf6_start (NMDevice *self) priv->ac_ip6_config = NULL; } - priv->rdisc = nm_lndp_rdisc_new (nm_device_get_ip_ifindex (self), nm_device_get_ip_iface (self)); + priv->rdisc = nm_lndp_rdisc_new (nm_device_get_ip_ifindex (self), ip_iface); if (!priv->rdisc) { - nm_log_err (LOGD_IP6, "Failed to start router discovery."); + nm_log_err (LOGD_IP6, "(%s): failed to start router discovery.", ip_iface); return FALSE; } /* ensure link local is ready... */ ret = linklocal6_start (self); - if (ret == NM_ACT_STAGE_RETURN_SUCCESS) addrconf6_start_with_link_ready (self); else From 3e60ff89531a02a4f55cac9231d970b47ad05963 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 19 Dec 2013 11:02:49 -0600 Subject: [PATCH 3/8] rdisc: log interface name in some messages --- src/rdisc/nm-lndp-rdisc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/rdisc/nm-lndp-rdisc.c b/src/rdisc/nm-lndp-rdisc.c index f94d82ac31..54e65e148a 100644 --- a/src/rdisc/nm-lndp-rdisc.c +++ b/src/rdisc/nm-lndp-rdisc.c @@ -190,7 +190,7 @@ send_rs (NMRDisc *rdisc) g_assert (!error); ndp_msg_ifindex_set (msg, rdisc->ifindex); - debug ("(%s): sending router solicitation: %d", rdisc->ifname, rdisc->ifindex); + debug ("(%s): sending router solicitation", rdisc->ifname); error = ndp_msg_send (priv->ndp, msg); if (error) @@ -346,7 +346,7 @@ check_timestamps (NMRDisc *rdisc, guint32 now, NMRDiscConfigMap changed) g_signal_emit_by_name (rdisc, NM_RDISC_CONFIG_CHANGED, changed); if (nextevent != never) { - debug ("Scheduling next now/lifetime check: %d seconds", (int) nextevent); + debug ("(%s): scheduling next now/lifetime check: %u seconds", rdisc->ifname, nextevent); priv->timeout_id = g_timeout_add_seconds (nextevent, timeout_cb, rdisc); } } @@ -454,7 +454,7 @@ receive_ra (struct ndp *ndp, struct ndp_msg *msg, gpointer user_data) * single time when the configuration is finished and updates can * come at any time. */ - debug ("Received router advertisement: %d at %d", rdisc->ifindex, (int) now); + debug ("(%s): received router advertisement at %u", rdisc->ifname, now); if (priv->send_rs_id) { g_source_remove (priv->send_rs_id); From d4684a7dc14cd1fa46f31b5d3544ec0bb24a3084 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 19 Dec 2013 11:05:13 -0600 Subject: [PATCH 4/8] rdisc: fix copy & paste error in detecting RA changes --- src/rdisc/nm-lndp-rdisc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rdisc/nm-lndp-rdisc.c b/src/rdisc/nm-lndp-rdisc.c index 54e65e148a..73e550f461 100644 --- a/src/rdisc/nm-lndp-rdisc.c +++ b/src/rdisc/nm-lndp-rdisc.c @@ -290,7 +290,7 @@ clean_servers (NMRDisc *rdisc, guint32 now, NMRDiscConfigMap *changed, guint32 * if (now >= expiry) { g_array_remove_index (rdisc->dns_servers, i--); - *changed |= NM_RDISC_CONFIG_ROUTES; + *changed |= NM_RDISC_CONFIG_DNS_SERVERS; } else if (now >= refresh) solicit (rdisc); else if (*nextevent > refresh) @@ -313,7 +313,7 @@ clean_domains (NMRDisc *rdisc, guint32 now, NMRDiscConfigMap *changed, guint32 * if (now >= expiry) { g_array_remove_index (rdisc->dns_domains, i--); - *changed |= NM_RDISC_CONFIG_ROUTES; + *changed |= NM_RDISC_CONFIG_DNS_DOMAINS; } else if (now >= refresh) solicit (rdisc); else if (*nextevent >=refresh) From 190fa1184347c2dcf22ba15c9f04cc799e69cdae Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 19 Dec 2013 11:06:52 -0600 Subject: [PATCH 5/8] rdisc: fix possible overflow in large RA option lifetimes If the address lifetime was near infinity (UINT_MAX) it could overflow a u32 and wrap back to small values. --- src/rdisc/nm-lndp-rdisc.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/rdisc/nm-lndp-rdisc.c b/src/rdisc/nm-lndp-rdisc.c index 73e550f461..aafa11775f 100644 --- a/src/rdisc/nm-lndp-rdisc.c +++ b/src/rdisc/nm-lndp-rdisc.c @@ -222,9 +222,9 @@ clean_gateways (NMRDisc *rdisc, guint32 now, NMRDiscConfigMap *changed, guint32 for (i = 0; i < rdisc->gateways->len; i++) { NMRDiscGateway *item = &g_array_index (rdisc->gateways, NMRDiscGateway, i); - guint32 expiry = item->timestamp + item->lifetime; + guint64 expiry = item->timestamp + item->lifetime; - if (item->lifetime == UINT_MAX) + if (item->lifetime == G_MAXUINT32) continue; if (now >= expiry) { @@ -242,9 +242,9 @@ clean_addresses (NMRDisc *rdisc, guint32 now, NMRDiscConfigMap *changed, guint32 for (i = 0; i < rdisc->addresses->len; i++) { NMRDiscAddress *item = &g_array_index (rdisc->addresses, NMRDiscAddress, i); - guint32 expiry = item->timestamp + item->lifetime; + guint64 expiry = item->timestamp + item->lifetime; - if (item->lifetime == UINT_MAX) + if (item->lifetime == G_MAXUINT32) continue; if (now >= expiry) { @@ -262,9 +262,9 @@ clean_routes (NMRDisc *rdisc, guint32 now, NMRDiscConfigMap *changed, guint32 *n for (i = 0; i < rdisc->routes->len; i++) { NMRDiscRoute *item = &g_array_index (rdisc->routes, NMRDiscRoute, i); - guint32 expiry = item->timestamp + item->lifetime; + guint64 expiry = item->timestamp + item->lifetime; - if (item->lifetime == UINT_MAX) + if (item->lifetime == G_MAXUINT32) continue; if (now >= expiry) { @@ -282,10 +282,10 @@ clean_servers (NMRDisc *rdisc, guint32 now, NMRDiscConfigMap *changed, guint32 * for (i = 0; i < rdisc->dns_servers->len; i++) { NMRDiscDNSServer *item = &g_array_index (rdisc->dns_servers, NMRDiscDNSServer, i); - guint32 expiry = item->timestamp + item->lifetime; - guint32 refresh = item->timestamp + item->lifetime / 2; + guint64 expiry = item->timestamp + item->lifetime; + guint64 refresh = item->timestamp + item->lifetime / 2; - if (item->lifetime == UINT_MAX) + if (item->lifetime == G_MAXUINT32) continue; if (now >= expiry) { @@ -305,10 +305,10 @@ clean_domains (NMRDisc *rdisc, guint32 now, NMRDiscConfigMap *changed, guint32 * for (i = 0; i < rdisc->dns_domains->len; i++) { NMRDiscDNSDomain *item = &g_array_index (rdisc->dns_domains, NMRDiscDNSDomain, i); - guint32 expiry = item->timestamp + item->lifetime; - guint32 refresh = item->timestamp + item->lifetime / 2; + guint64 expiry = item->timestamp + item->lifetime; + guint64 refresh = item->timestamp + item->lifetime / 2; - if (item->lifetime == UINT_MAX) + if (item->lifetime == G_MAXUINT32) continue; if (now >= expiry) { @@ -327,7 +327,7 @@ static void check_timestamps (NMRDisc *rdisc, guint32 now, NMRDiscConfigMap changed) { NMLNDPRDiscPrivate *priv = NM_LNDP_RDISC_GET_PRIVATE (rdisc); - /* Use a magic date in distant enough future as there's no guint32 max macro. */ + /* Use a magic date in the distant future (~68 years) */ guint32 never = G_MAXINT32; guint32 nextevent = never; From 773d4e4dc8320bb380d13f8be74ab26702a4191e Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 19 Dec 2013 11:51:53 -0600 Subject: [PATCH 6/8] rdisc: don't leak DNSSL domains if they aren't added If the new DNSSL domain was not used, becuase it was already in the list, the domain string would be leaked. --- src/rdisc/nm-lndp-rdisc.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/rdisc/nm-lndp-rdisc.c b/src/rdisc/nm-lndp-rdisc.c index aafa11775f..144182aa1c 100644 --- a/src/rdisc/nm-lndp-rdisc.c +++ b/src/rdisc/nm-lndp-rdisc.c @@ -160,20 +160,23 @@ add_server (NMRDisc *rdisc, const NMRDiscDNSServer *new) return TRUE; } +/* Copies new->domain if 'new' is added to the dns_domains list */ static gboolean add_domain (NMRDisc *rdisc, const NMRDiscDNSDomain *new) { + NMRDiscDNSDomain *item; int i; for (i = 0; i < rdisc->dns_domains->len; i++) { - NMRDiscDNSDomain *item = &g_array_index (rdisc->dns_domains, NMRDiscDNSDomain, i); + item = &g_array_index (rdisc->dns_domains, NMRDiscDNSDomain, i); if (!g_strcmp0 (item->domain, new->domain)) return FALSE; } g_array_insert_val (rdisc->dns_domains, i, *new); - + item = &g_array_index (rdisc->dns_domains, NMRDiscDNSDomain, i); + item->domain = g_strdup (new->domain); return TRUE; } @@ -312,6 +315,7 @@ clean_domains (NMRDisc *rdisc, guint32 now, NMRDiscConfigMap *changed, guint32 * continue; if (now >= expiry) { + g_free (item->domain); g_array_remove_index (rdisc->dns_domains, i--); *changed |= NM_RDISC_CONFIG_DNS_DOMAINS; } else if (now >= refresh) @@ -575,7 +579,7 @@ receive_ra (struct ndp *ndp, struct ndp_msg *msg, gpointer user_data) NMRDiscDNSDomain dns_domain; memset (&dns_domain, 0, sizeof (dns_domain)); - dns_domain.domain = g_strdup (domain); + dns_domain.domain = domain; dns_domain.timestamp = now; dns_domain.lifetime = ndp_msg_opt_rdnss_lifetime (msg, offset); /* Pad the lifetime somewhat to give a bit of slack in cases From 3f654dc6e6487c02a7c157c117fcb16bbed6b184 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 19 Dec 2013 11:56:00 -0600 Subject: [PATCH 7/8] rdisc: ensure RDNSS and DNSSL lifetimes are updated (rh #1044757) (bgo #720760) The DNS server and domain timestamps and lifetimes weren't updated when a new RA was received. When half the lifetime for either of them had passed, clean_dns_servers() and clean_domains() request a Router Solicitation to ensure the DNS information does not expire. This obviously results in a new Router Advertisement, but since the timestamps don't get updated, clean_dns_servers() and clean_domains() simply request another solicitation because 'now' is still greater than half the old lifetime. This casues another solicit request, which causes another RA, which... etc. https://bugzilla.redhat.com/show_bug.cgi?id=1044757 https://bugzilla.gnome.org/show_bug.cgi?id=720760 --- src/rdisc/nm-lndp-rdisc.c | 39 ++++++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/src/rdisc/nm-lndp-rdisc.c b/src/rdisc/nm-lndp-rdisc.c index 144182aa1c..6afe60c7f4 100644 --- a/src/rdisc/nm-lndp-rdisc.c +++ b/src/rdisc/nm-lndp-rdisc.c @@ -144,25 +144,31 @@ add_route (NMRDisc *rdisc, const NMRDiscRoute *new) } static gboolean -add_server (NMRDisc *rdisc, const NMRDiscDNSServer *new) +add_dns_server (NMRDisc *rdisc, const NMRDiscDNSServer *new) { int i; for (i = 0; i < rdisc->dns_servers->len; i++) { NMRDiscDNSServer *item = &g_array_index (rdisc->dns_servers, NMRDiscDNSServer, i); - if (IN6_ARE_ADDR_EQUAL (&item->address, &new->address)) - return FALSE; + if (IN6_ARE_ADDR_EQUAL (&item->address, &new->address)) { + gboolean changed = item->timestamp != new->timestamp || + item->lifetime != new->lifetime; + if (changed) { + item->timestamp = new->timestamp; + item->lifetime = new->lifetime; + } + return changed; + } } g_array_insert_val (rdisc->dns_servers, i, *new); - return TRUE; } /* Copies new->domain if 'new' is added to the dns_domains list */ static gboolean -add_domain (NMRDisc *rdisc, const NMRDiscDNSDomain *new) +add_dns_domain (NMRDisc *rdisc, const NMRDiscDNSDomain *new) { NMRDiscDNSDomain *item; int i; @@ -170,8 +176,15 @@ add_domain (NMRDisc *rdisc, const NMRDiscDNSDomain *new) for (i = 0; i < rdisc->dns_domains->len; i++) { item = &g_array_index (rdisc->dns_domains, NMRDiscDNSDomain, i); - if (!g_strcmp0 (item->domain, new->domain)) - return FALSE; + if (!g_strcmp0 (item->domain, new->domain)) { + gboolean changed = item->timestamp != new->timestamp || + item->lifetime != new->lifetime; + if (changed) { + item->timestamp = new->timestamp; + item->lifetime = new->lifetime; + } + return changed; + } } g_array_insert_val (rdisc->dns_domains, i, *new); @@ -279,7 +292,7 @@ clean_routes (NMRDisc *rdisc, guint32 now, NMRDiscConfigMap *changed, guint32 *n } static void -clean_servers (NMRDisc *rdisc, guint32 now, NMRDiscConfigMap *changed, guint32 *nextevent) +clean_dns_servers (NMRDisc *rdisc, guint32 now, NMRDiscConfigMap *changed, guint32 *nextevent) { int i; @@ -302,7 +315,7 @@ clean_servers (NMRDisc *rdisc, guint32 now, NMRDiscConfigMap *changed, guint32 * } static void -clean_domains (NMRDisc *rdisc, guint32 now, NMRDiscConfigMap *changed, guint32 *nextevent) +clean_dns_domains (NMRDisc *rdisc, guint32 now, NMRDiscConfigMap *changed, guint32 *nextevent) { int i; @@ -343,8 +356,8 @@ check_timestamps (NMRDisc *rdisc, guint32 now, NMRDiscConfigMap changed) clean_gateways (rdisc, now, &changed, &nextevent); clean_addresses (rdisc, now, &changed, &nextevent); clean_routes (rdisc, now, &changed, &nextevent); - clean_servers (rdisc, now, &changed, &nextevent); - clean_domains (rdisc, now, &changed, &nextevent); + clean_dns_servers (rdisc, now, &changed, &nextevent); + clean_dns_domains (rdisc, now, &changed, &nextevent); if (changed) g_signal_emit_by_name (rdisc, NM_RDISC_CONFIG_CHANGED, changed); @@ -567,7 +580,7 @@ receive_ra (struct ndp *ndp, struct ndp_msg *msg, gpointer user_data) */ if (dns_server.lifetime && dns_server.lifetime < 7200) dns_server.lifetime = 7200; - if (add_server (rdisc, &dns_server)) + if (add_dns_server (rdisc, &dns_server)) changed |= NM_RDISC_CONFIG_DNS_SERVERS; } } @@ -589,7 +602,7 @@ receive_ra (struct ndp *ndp, struct ndp_msg *msg, gpointer user_data) */ if (dns_domain.lifetime && dns_domain.lifetime < 7200) dns_domain.lifetime = 7200; - if (add_domain (rdisc, &dns_domain)) + if (add_dns_domain (rdisc, &dns_domain)) changed |= NM_RDISC_CONFIG_DNS_DOMAINS; } } From b705d7f50e827d55419e9da9d8b8ae5ff027c366 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 19 Dec 2013 12:05:54 -0600 Subject: [PATCH 8/8] rdisc: don't add new RDNSS and DNSSL options with zero lifetime A lifetime of 0 means that the domain or server should no longer be used, so if we get an RA with a zero-lifetime DNS server or domain that we haven't seen before, don't bother adding it to the list. DNS servers and domains that are already known and become zero lifetime in the next RA are already correctly handled by clean_dns_servers() and clean_domains(). --- src/rdisc/nm-lndp-rdisc.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/rdisc/nm-lndp-rdisc.c b/src/rdisc/nm-lndp-rdisc.c index 6afe60c7f4..04b52067fc 100644 --- a/src/rdisc/nm-lndp-rdisc.c +++ b/src/rdisc/nm-lndp-rdisc.c @@ -162,6 +162,10 @@ add_dns_server (NMRDisc *rdisc, const NMRDiscDNSServer *new) } } + /* DNS server should no longer be used */ + if (new->lifetime == 0) + return FALSE; + g_array_insert_val (rdisc->dns_servers, i, *new); return TRUE; } @@ -187,6 +191,10 @@ add_dns_domain (NMRDisc *rdisc, const NMRDiscDNSDomain *new) } } + /* Domain should no longer be used */ + if (new->lifetime == 0) + return FALSE; + g_array_insert_val (rdisc->dns_domains, i, *new); item = &g_array_index (rdisc->dns_domains, NMRDiscDNSDomain, i); item->domain = g_strdup (new->domain);