From 8177b0c397ad2d53804d446d7e5a0332234df14a Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Thu, 11 Oct 2018 13:22:08 +0200
Subject: [PATCH 1/7] ndisc: ignore addresses with preferred lifetime larger
 than lifetime

Previously, we would coerce the value so that preferred is the same
as lifetime. However, RFC4862 5.5.3.c) says:

  c)  If the preferred lifetime is greater than the valid lifetime,
    silently ignore the Prefix Information option.  A node MAY wish to
    log a system management error in this case.

See-also: https://tools.ietf.org/search/rfc4862#section-5.5.3
(cherry picked from commit 43c3c259c8e2fd4aa5416b79efd47a72cca790ec)
(cherry picked from commit eff9e161cb056c544763dd2f45de7371ac46b8bd)
(cherry picked from commit dbfa7950cfab63491d2a237311616f98e38ab73c)
---
 src/ndisc/nm-lndp-ndisc.c | 8 ++++----
 src/ndisc/nm-ndisc.c      | 1 +
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/ndisc/nm-lndp-ndisc.c b/src/ndisc/nm-lndp-ndisc.c
index 8bb1812764..48a17bdf4f 100644
--- a/src/ndisc/nm-lndp-ndisc.c
+++ b/src/ndisc/nm-lndp-ndisc.c
@@ -221,10 +221,10 @@ receive_ra (struct ndp *ndp, struct ndp_msg *msg, gpointer user_data)
 				.preferred = ndp_msg_opt_prefix_preferred_time (msg, offset),
 			};
 
-			if (address.preferred > address.lifetime)
-				address.preferred = address.lifetime;
-			if (nm_ndisc_complete_and_add_address (ndisc, &address))
-				changed |= NM_NDISC_CONFIG_ADDRESSES;
+			if (address.preferred <= address.lifetime) {
+				if (nm_ndisc_complete_and_add_address (ndisc, &address))
+					changed |= NM_NDISC_CONFIG_ADDRESSES;
+			}
 		}
 	}
 	ndp_msg_opt_for_each_offset(offset, msg, NDP_MSG_OPT_ROUTE) {
diff --git a/src/ndisc/nm-ndisc.c b/src/ndisc/nm-ndisc.c
index e86cfdfbe0..87e5e0733a 100644
--- a/src/ndisc/nm-ndisc.c
+++ b/src/ndisc/nm-ndisc.c
@@ -416,6 +416,7 @@ nm_ndisc_add_address (NMNDisc *ndisc, const NMNDiscAddress *new)
 	nm_assert (new->timestamp > 0 && new->timestamp < G_MAXINT32);
 	nm_assert (!IN6_IS_ADDR_UNSPECIFIED (&new->address));
 	nm_assert (!IN6_IS_ADDR_LINKLOCAL (&new->address));
+	nm_assert (new->preferred <= new->lifetime);
 
 	for (i = 0; i < rdata->addresses->len; i++) {
 		NMNDiscAddress *item = &g_array_index (rdata->addresses, NMNDiscAddress, i);

From aeca2fb4b123be20d4cc112456a41778f984c3cc Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Fri, 12 Oct 2018 11:48:34 +0200
Subject: [PATCH 2/7] ndisc: ensure we skip unspecified IPv6 address in
 ndisc_set_router_config()

Later, nm_ndisc_add_address() asserts that the address is not an
unspecified address. Skip it, just to be sure.

(cherry picked from commit 700b04d0deded1f8b8267e13202d3c29f8748e14)
(cherry picked from commit e0e698e463ef244223d4af500ba7a688ba976c86)
(cherry picked from commit 547dcacbfbf317b44f2ddd552339207df39d2d1c)
---
 src/devices/nm-device.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index 7af5de925a..d45f1bb07b 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -2645,7 +2645,8 @@ ndisc_set_router_config (NMNDisc *ndisc, NMDevice *self)
 		guint32 lifetime, preferred;
 		gint32 base;
 
-		if (IN6_IS_ADDR_LINKLOCAL (&addr->address))
+		if (   IN6_IS_ADDR_UNSPECIFIED (&addr->address)
+		    || IN6_IS_ADDR_LINKLOCAL (&addr->address))
 			continue;
 
 		if (   addr->n_ifa_flags & IFA_F_TENTATIVE

From d4b67b9441a61f03e8796972f2e9741be77799bb Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Thu, 11 Oct 2018 13:40:58 +0200
Subject: [PATCH 3/7] ndisc: only generate address interface identifer after
 checking existing prefix

RFC4862 5.5.3, points d) and e) make it clear, that the list of
addresses should be compared based on the prefix.

  d)  If the prefix advertised is not equal to the prefix of an
    address configured by stateless autoconfiguration already in the
    list of addresses associated with the interface (where "equal"
    means the two prefix lengths are the same and the first prefix-
    length bits of the prefixes are identical), and if the Valid
    Lifetime is not 0, form an address (and add it to the list) by
    combining the advertised prefix with an interface identifier of
    the link as follows:

That means, we should not initialize the interface identifier first
(via complete_address()) and then search for the full address.

See-also: https://tools.ietf.org/search/rfc4862#section-5.5.3
(cherry picked from commit 23c417854a943eb796a8c06e7d94b02d18f19691)
(cherry picked from commit ac5669633c44f6da3ec417282d59d71f229c2b7a)
(cherry picked from commit b2f084a8ae927d058818d7126e4536d3b36a23bd)
---
 src/ndisc/nm-ndisc-private.h |  2 +-
 src/ndisc/nm-ndisc.c         | 63 ++++++++++++++++++++++++------------
 2 files changed, 43 insertions(+), 22 deletions(-)

diff --git a/src/ndisc/nm-ndisc-private.h b/src/ndisc/nm-ndisc-private.h
index bbecb01a43..cdf8dc282a 100644
--- a/src/ndisc/nm-ndisc-private.h
+++ b/src/ndisc/nm-ndisc-private.h
@@ -40,7 +40,7 @@ void nm_ndisc_ra_received (NMNDisc *ndisc, gint32 now, NMNDiscConfigMap changed)
 void nm_ndisc_rs_received (NMNDisc *ndisc);
 
 gboolean nm_ndisc_add_gateway              (NMNDisc *ndisc, const NMNDiscGateway *new);
-gboolean nm_ndisc_complete_and_add_address (NMNDisc *ndisc, NMNDiscAddress *new);
+gboolean nm_ndisc_complete_and_add_address (NMNDisc *ndisc, const NMNDiscAddress *new);
 gboolean nm_ndisc_add_route                (NMNDisc *ndisc, const NMNDiscRoute *new);
 gboolean nm_ndisc_add_dns_server           (NMNDisc *ndisc, const NMNDiscDNSServer *new);
 gboolean nm_ndisc_add_dns_domain           (NMNDisc *ndisc, const NMNDiscDNSDomain *new);
diff --git a/src/ndisc/nm-ndisc.c b/src/ndisc/nm-ndisc.c
index 87e5e0733a..6b580ccc98 100644
--- a/src/ndisc/nm-ndisc.c
+++ b/src/ndisc/nm-ndisc.c
@@ -406,10 +406,11 @@ complete_address (NMNDisc *ndisc, NMNDiscAddress *addr)
 }
 
 static gboolean
-nm_ndisc_add_address (NMNDisc *ndisc, const NMNDiscAddress *new)
+nm_ndisc_add_address (NMNDisc *ndisc, const NMNDiscAddress *new, gboolean from_ra)
 {
 	NMNDiscPrivate *priv = NM_NDISC_GET_PRIVATE (ndisc);
 	NMNDiscDataInternal *rdata = &priv->rdata;
+	NMNDiscAddress new2;
 	guint i;
 
 	nm_assert (new);
@@ -421,20 +422,33 @@ nm_ndisc_add_address (NMNDisc *ndisc, const NMNDiscAddress *new)
 	for (i = 0; i < rdata->addresses->len; i++) {
 		NMNDiscAddress *item = &g_array_index (rdata->addresses, NMNDiscAddress, i);
 
-		if (IN6_ARE_ADDR_EQUAL (&item->address, &new->address)) {
-			if (new->lifetime == 0) {
-				g_array_remove_index (rdata->addresses, i);
-				return TRUE;
-			}
+		if (from_ra) {
+			/* RFC4862 5.5.3.d, we find an existing address with the same prefix.
+			 * (note that all prefixes at this point have implicity length /64). */
+			if (memcmp (&item->address, &new->address, 8) != 0)
+				continue;
+		} else {
+			if (!IN6_ARE_ADDR_EQUAL (&item->address, &new->address))
+				continue;
+		}
 
-			if (   item->dad_counter == new->dad_counter
-			    && get_expiry (item) == get_expiry (new)
-			    && get_expiry_preferred (item) == get_expiry_preferred (new))
-				return FALSE;
-
-			*item = *new;
+		if (new->lifetime == 0) {
+			g_array_remove_index (rdata->addresses, i);
 			return TRUE;
 		}
+
+		if (   get_expiry (item) == get_expiry (new)
+		    && get_expiry_preferred (item) == get_expiry_preferred (new)
+		    && (   from_ra
+		        || item->dad_counter == new->dad_counter))
+			return FALSE;
+
+		if (!from_ra)
+			item->dad_counter = new->dad_counter;
+		item->timestamp = new->timestamp;
+		item->lifetime = new->lifetime;
+		item->preferred = new->preferred;
+		return TRUE;
 	}
 
 	/* we create at most max_addresses autoconf addresses. This is different from
@@ -445,18 +459,25 @@ nm_ndisc_add_address (NMNDisc *ndisc, const NMNDiscAddress *new)
 	    && rdata->addresses->len >= priv->max_addresses)
 		return FALSE;
 
-	if (new->lifetime)
-		g_array_append_val (rdata->addresses, *new);
-	return !!new->lifetime;
+	if (new->lifetime == 0)
+		return FALSE;
+
+	if (from_ra) {
+		new2 = *new;
+		new2.dad_counter = 0;
+		if (!complete_address (ndisc, &new2))
+			return FALSE;
+		new = &new2;
+	}
+
+	g_array_append_val (rdata->addresses, *new);
+	return TRUE;
 }
 
 gboolean
-nm_ndisc_complete_and_add_address (NMNDisc *ndisc, NMNDiscAddress *new)
+nm_ndisc_complete_and_add_address (NMNDisc *ndisc, const NMNDiscAddress *new)
 {
-	if (!complete_address (ndisc, new))
-		return FALSE;
-
-	return nm_ndisc_add_address (ndisc, new);
+	return nm_ndisc_add_address (ndisc, new, TRUE);
 }
 
 gboolean
@@ -754,7 +775,7 @@ nm_ndisc_set_config (NMNDisc *ndisc,
 	guint i;
 
 	for (i = 0; i < addresses->len; i++) {
-		if (nm_ndisc_add_address (ndisc, &g_array_index (addresses, NMNDiscAddress, i)))
+		if (nm_ndisc_add_address (ndisc, &g_array_index (addresses, NMNDiscAddress, i), FALSE))
 			changed = TRUE;
 	}
 

From 3d99992593e070e91fc762d73d66f68131e63b0c Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Thu, 11 Oct 2018 13:52:03 +0200
Subject: [PATCH 4/7] ndisc: minor refactoring loop in nm_ndisc_add_address()

No change in behavior. Just don't do so much work inside
the deeper nesting of the loop.

(cherry picked from commit 9d0a138ef05544954de49a8056f47e34c6bf3c5a)
(cherry picked from commit 3cecb4d0186391dcd09fb6bf70afa96fc16ebf3d)
(cherry picked from commit 669e004299605b76ef30fe7ee7f4728f59f65acf)
---
 src/ndisc/nm-ndisc.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/src/ndisc/nm-ndisc.c b/src/ndisc/nm-ndisc.c
index 6b580ccc98..a516c900cd 100644
--- a/src/ndisc/nm-ndisc.c
+++ b/src/ndisc/nm-ndisc.c
@@ -411,6 +411,7 @@ nm_ndisc_add_address (NMNDisc *ndisc, const NMNDiscAddress *new, gboolean from_r
 	NMNDiscPrivate *priv = NM_NDISC_GET_PRIVATE (ndisc);
 	NMNDiscDataInternal *rdata = &priv->rdata;
 	NMNDiscAddress new2;
+	NMNDiscAddress *existing = NULL;
 	guint i;
 
 	nm_assert (new);
@@ -425,29 +426,35 @@ nm_ndisc_add_address (NMNDisc *ndisc, const NMNDiscAddress *new, gboolean from_r
 		if (from_ra) {
 			/* RFC4862 5.5.3.d, we find an existing address with the same prefix.
 			 * (note that all prefixes at this point have implicity length /64). */
-			if (memcmp (&item->address, &new->address, 8) != 0)
-				continue;
+			if (memcmp (&item->address, &new->address, 8) == 0) {
+				existing = item;
+				break;
+			}
 		} else {
-			if (!IN6_ARE_ADDR_EQUAL (&item->address, &new->address))
-				continue;
+			if (IN6_ARE_ADDR_EQUAL (&item->address, &new->address)) {
+				existing = item;
+				break;
+			}
 		}
+	}
 
+	if (existing) {
 		if (new->lifetime == 0) {
 			g_array_remove_index (rdata->addresses, i);
 			return TRUE;
 		}
 
-		if (   get_expiry (item) == get_expiry (new)
-		    && get_expiry_preferred (item) == get_expiry_preferred (new)
+		if (   get_expiry (existing) == get_expiry (new)
+		    && get_expiry_preferred (existing) == get_expiry_preferred (new)
 		    && (   from_ra
-		        || item->dad_counter == new->dad_counter))
+		        || existing->dad_counter == new->dad_counter))
 			return FALSE;
 
 		if (!from_ra)
-			item->dad_counter = new->dad_counter;
-		item->timestamp = new->timestamp;
-		item->lifetime = new->lifetime;
-		item->preferred = new->preferred;
+			existing->dad_counter = new->dad_counter;
+		existing->timestamp = new->timestamp;
+		existing->lifetime = new->lifetime;
+		existing->preferred = new->preferred;
 		return TRUE;
 	}
 

From 2e365f638bbf2353e060e0c55b803889a57baca3 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Fri, 12 Oct 2018 13:05:51 +0200
Subject: [PATCH 5/7] ndisc: handle integer overflows better for lifetime
 handling

we use get_expiry() to compare two lifetimes. Note, that previously,
it would correctly truncate the calculated expiry at G_MAXINT32-1.

However, that means, that two different lifetimes that both lie
more than 68 years in the future would compare equal.

Fix that, but extending the range to int64, so that no overflow
can happen.

(cherry picked from commit b086535cb7f28a495a742c64ed4e212175f19860)
(cherry picked from commit fe60843232a2bbd6e78eec5b25a1a5f8896566b2)
(cherry picked from commit 42e61a8cc8fb56e7dd625edcec470aa090747839)
---
 src/ndisc/nm-ndisc.c | 112 ++++++++++++++++++++-----------------------
 src/ndisc/nm-ndisc.h |   3 ++
 2 files changed, 56 insertions(+), 59 deletions(-)

diff --git a/src/ndisc/nm-ndisc.c b/src/ndisc/nm-ndisc.c
index a516c900cd..4ac3be9b8d 100644
--- a/src/ndisc/nm-ndisc.c
+++ b/src/ndisc/nm-ndisc.c
@@ -124,53 +124,66 @@ _preference_to_priority (NMIcmpv6RouterPref pref)
 
 /*****************************************************************************/
 
-static gint32
+/* we rely on the fact, that _EXPIRY_INFINITY > any other valid gint64 timestamps. */
+#define _EXPIRY_INFINITY G_MAXINT64
+
+static gint64
 get_expiry_time (guint32 timestamp, guint32 lifetime)
 {
-	gint64 t;
-
-	/* timestamp is supposed to come from nm_utils_get_monotonic_timestamp_s().
-	 * It is expected to be within a certain range. */
 	nm_assert (timestamp > 0);
 	nm_assert (timestamp <= G_MAXINT32);
 
 	if (lifetime == NM_NDISC_INFINITY)
-		return G_MAXINT32;
-
-	t = (gint64) timestamp + (gint64) lifetime;
-	return CLAMP (t, 0, G_MAXINT32 - 1);
+		return _EXPIRY_INFINITY;
+	return ((gint64) timestamp) + ((gint64) lifetime);
 }
 
 #define get_expiry(item) \
 	({ \
 		typeof (item) _item = (item); \
 		nm_assert (_item); \
-		get_expiry_time ((_item->timestamp), (_item->lifetime)); \
+		get_expiry_time (_item->timestamp, _item->lifetime); \
 	})
 
 #define get_expiry_half(item) \
 	({ \
 		typeof (item) _item = (item); \
 		nm_assert (_item); \
-		get_expiry_time ((_item->timestamp),\
-		                 (_item->lifetime) == NM_NDISC_INFINITY \
-		                   ? NM_NDISC_INFINITY \
-		                   : (_item->lifetime) / 2); \
+		(_item->lifetime == NM_NDISC_INFINITY) \
+		  ? _EXPIRY_INFINITY \
+		  : get_expiry_time (_item->timestamp, _item->lifetime / 2); \
 	})
 
 #define get_expiry_preferred(item) \
 	({ \
 		typeof (item) _item = (item); \
 		nm_assert (_item); \
-		get_expiry_time ((_item->timestamp), (_item->preferred)); \
+		get_expiry_time (_item->timestamp, _item->preferred); \
 	})
 
+static gboolean
+expiry_next (gint32 now_s, gint64 expiry_timestamp, gint32 *nextevent)
+{
+	gint32 e;
+
+	if (expiry_timestamp == _EXPIRY_INFINITY)
+		return TRUE;
+	e = MIN (expiry_timestamp, ((gint64) (G_MAXINT32 - 1)));
+	if (now_s >= e)
+		return FALSE;
+	if (nextevent) {
+		if (*nextevent > e)
+			*nextevent = e;
+	}
+	return TRUE;
+}
+
 static const char *
-_get_exp (char *buf, gsize buf_size, gint64 now_ns, gint32 expiry_time)
+_get_exp (char *buf, gsize buf_size, gint64 now_ns, gint64 expiry_time)
 {
 	int l;
 
-	if (expiry_time == G_MAXINT32)
+	if (expiry_time == _EXPIRY_INFINITY)
 		return "permanent";
 	l = g_snprintf (buf, buf_size,
 	                "%.4f",
@@ -1027,17 +1040,12 @@ clean_gateways (NMNDisc *ndisc, gint32 now, NMNDiscConfigMap *changed, gint32 *n
 	for (i = 0; i < rdata->gateways->len; ) {
 		NMNDiscGateway *item = &g_array_index (rdata->gateways, NMNDiscGateway, i);
 
-		if (item->lifetime != NM_NDISC_INFINITY) {
-			gint32 expiry = get_expiry (item);
-
-			if (now >= expiry) {
-				g_array_remove_index (rdata->gateways, i);
-				*changed |= NM_NDISC_CONFIG_GATEWAYS;
-				continue;
-			}
-			if (*nextevent > expiry)
-				*nextevent = expiry;
+		if (!expiry_next (now, get_expiry (item), nextevent)) {
+			g_array_remove_index (rdata->gateways, i);
+			*changed |= NM_NDISC_CONFIG_GATEWAYS;
+			continue;
 		}
+
 		i++;
 	}
 
@@ -1055,17 +1063,12 @@ clean_addresses (NMNDisc *ndisc, gint32 now, NMNDiscConfigMap *changed, gint32 *
 	for (i = 0; i < rdata->addresses->len; ) {
 		const NMNDiscAddress *item = &g_array_index (rdata->addresses, NMNDiscAddress, i);
 
-		if (item->lifetime != NM_NDISC_INFINITY) {
-			gint32 expiry = get_expiry (item);
-
-			if (now >= expiry) {
-				g_array_remove_index (rdata->addresses, i);
-				*changed |= NM_NDISC_CONFIG_ADDRESSES;
-				continue;
-			}
-			if (*nextevent > expiry)
-				*nextevent = expiry;
+		if (!expiry_next (now, get_expiry (item), nextevent)) {
+			g_array_remove_index (rdata->addresses, i);
+			*changed |= NM_NDISC_CONFIG_ADDRESSES;
+			continue;
 		}
+
 		i++;
 	}
 }
@@ -1081,17 +1084,12 @@ clean_routes (NMNDisc *ndisc, gint32 now, NMNDiscConfigMap *changed, gint32 *nex
 	for (i = 0; i < rdata->routes->len; ) {
 		NMNDiscRoute *item = &g_array_index (rdata->routes, NMNDiscRoute, i);
 
-		if (item->lifetime != NM_NDISC_INFINITY) {
-			gint32 expiry = get_expiry (item);
-
-			if (now >= expiry) {
-				g_array_remove_index (rdata->routes, i);
-				*changed |= NM_NDISC_CONFIG_ROUTES;
-				continue;
-			}
-			if (*nextevent > expiry)
-				*nextevent = expiry;
+		if (!expiry_next (now, get_expiry (item), nextevent)) {
+			g_array_remove_index (rdata->routes, i);
+			*changed |= NM_NDISC_CONFIG_ROUTES;
+			continue;
 		}
+
 		i++;
 	}
 }
@@ -1106,18 +1104,16 @@ clean_dns_servers (NMNDisc *ndisc, gint32 now, NMNDiscConfigMap *changed, gint32
 
 	for (i = 0; i < rdata->dns_servers->len; ) {
 		NMNDiscDNSServer *item = &g_array_index (rdata->dns_servers, NMNDiscDNSServer, i);
+		gint64 refresh;
 
-		if (item->lifetime != NM_NDISC_INFINITY) {
-			gint32 expiry = get_expiry (item);
-			gint32 refresh;
-
-			if (now >= expiry) {
+		refresh = get_expiry_half (item);
+		if (refresh != _EXPIRY_INFINITY) {
+			if (!expiry_next (now, get_expiry (item), NULL)) {
 				g_array_remove_index (rdata->dns_servers, i);
 				*changed |= NM_NDISC_CONFIG_DNS_SERVERS;
 				continue;
 			}
 
-			refresh = get_expiry_half (item);
 			if (now >= refresh)
 				solicit_routers (ndisc);
 			else if (*nextevent > refresh)
@@ -1137,18 +1133,16 @@ clean_dns_domains (NMNDisc *ndisc, gint32 now, NMNDiscConfigMap *changed, gint32
 
 	for (i = 0; i < rdata->dns_domains->len; ) {
 		NMNDiscDNSDomain *item = &g_array_index (rdata->dns_domains, NMNDiscDNSDomain, i);
+		gint64 refresh;
 
-		if (item->lifetime != NM_NDISC_INFINITY) {
-			gint32 expiry = get_expiry (item);
-			gint32 refresh;
-
-			if (now >= expiry) {
+		refresh = get_expiry_half (item);
+		if (refresh != _EXPIRY_INFINITY) {
+			if (!expiry_next (now, get_expiry (item), NULL)) {
 				g_array_remove_index (rdata->dns_domains, i);
 				*changed |= NM_NDISC_CONFIG_DNS_DOMAINS;
 				continue;
 			}
 
-			refresh = get_expiry_half (item);
 			if (now >= refresh)
 				solicit_routers (ndisc);
 			else if (*nextevent > refresh)
diff --git a/src/ndisc/nm-ndisc.h b/src/ndisc/nm-ndisc.h
index 3cbb8f5000..faca372dcc 100644
--- a/src/ndisc/nm-ndisc.h
+++ b/src/ndisc/nm-ndisc.h
@@ -55,6 +55,9 @@ typedef enum {
 	NM_NDISC_DHCP_LEVEL_MANAGED
 } NMNDiscDHCPLevel;
 
+/* we rely on the fact that NM_NDISC_INFINITY is the largest possible
+ * time duration (G_MAXUINT32) and that the range of finite values
+ * goes from 0 to G_MAXUINT32-1. */
 #define NM_NDISC_INFINITY  G_MAXUINT32
 
 struct _NMNDiscGateway {

From cc16d893954172829837afd48f0fbf12a5df89a2 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Thu, 11 Oct 2018 14:57:38 +0200
Subject: [PATCH 6/7] ndisc: fix updating address lifetime on Router
 Announcement according to RFC4862

This is a denial-of-service protection, where a malicious router
advertisement can expire the addresses.

See-also: https://github.com/systemd/systemd/commit/6554550f35a7976f9110aff94743d3576d5f02dd
See-also: https://tools.ietf.org/search/rfc4862#section-5.5.3

https://bugs.launchpad.net/ubuntu/+source/network-manager/+bug/1796622
(cherry picked from commit 27be3e03382deb3da29e0c31dc6cb871bd6be0e8)
(cherry picked from commit 8e2ccd3921427d6d589b0695b2b219bdb84b1121)
(cherry picked from commit 451bf6e275517e139ee96ee2fa4acc747d10561c)
---
 src/ndisc/nm-fake-ndisc.c         |  2 +-
 src/ndisc/nm-lndp-ndisc.c         |  2 +-
 src/ndisc/nm-ndisc-private.h      |  2 +-
 src/ndisc/nm-ndisc.c              | 65 +++++++++++++++++++++++++++----
 src/ndisc/tests/test-ndisc-fake.c |  7 ++--
 5 files changed, 64 insertions(+), 14 deletions(-)

diff --git a/src/ndisc/nm-fake-ndisc.c b/src/ndisc/nm-fake-ndisc.c
index 15abee88f1..6e9a72c6f4 100644
--- a/src/ndisc/nm-fake-ndisc.c
+++ b/src/ndisc/nm-fake-ndisc.c
@@ -296,7 +296,7 @@ receive_ra (gpointer user_data)
 				.dad_counter = 0,
 			};
 
-			if (nm_ndisc_complete_and_add_address (ndisc, &address))
+			if (nm_ndisc_complete_and_add_address (ndisc, &address, now))
 				changed |= NM_NDISC_CONFIG_ADDRESSES;
 		}
 	}
diff --git a/src/ndisc/nm-lndp-ndisc.c b/src/ndisc/nm-lndp-ndisc.c
index 48a17bdf4f..9f427d4d59 100644
--- a/src/ndisc/nm-lndp-ndisc.c
+++ b/src/ndisc/nm-lndp-ndisc.c
@@ -222,7 +222,7 @@ receive_ra (struct ndp *ndp, struct ndp_msg *msg, gpointer user_data)
 			};
 
 			if (address.preferred <= address.lifetime) {
-				if (nm_ndisc_complete_and_add_address (ndisc, &address))
+				if (nm_ndisc_complete_and_add_address (ndisc, &address, now))
 					changed |= NM_NDISC_CONFIG_ADDRESSES;
 			}
 		}
diff --git a/src/ndisc/nm-ndisc-private.h b/src/ndisc/nm-ndisc-private.h
index cdf8dc282a..ae03c0ee43 100644
--- a/src/ndisc/nm-ndisc-private.h
+++ b/src/ndisc/nm-ndisc-private.h
@@ -40,7 +40,7 @@ void nm_ndisc_ra_received (NMNDisc *ndisc, gint32 now, NMNDiscConfigMap changed)
 void nm_ndisc_rs_received (NMNDisc *ndisc);
 
 gboolean nm_ndisc_add_gateway              (NMNDisc *ndisc, const NMNDiscGateway *new);
-gboolean nm_ndisc_complete_and_add_address (NMNDisc *ndisc, const NMNDiscAddress *new);
+gboolean nm_ndisc_complete_and_add_address (NMNDisc *ndisc, const NMNDiscAddress *new, gint32 now_s);
 gboolean nm_ndisc_add_route                (NMNDisc *ndisc, const NMNDiscRoute *new);
 gboolean nm_ndisc_add_dns_server           (NMNDisc *ndisc, const NMNDiscDNSServer *new);
 gboolean nm_ndisc_add_dns_domain           (NMNDisc *ndisc, const NMNDiscDNSDomain *new);
diff --git a/src/ndisc/nm-ndisc.c b/src/ndisc/nm-ndisc.c
index 4ac3be9b8d..44c8a09d3d 100644
--- a/src/ndisc/nm-ndisc.c
+++ b/src/ndisc/nm-ndisc.c
@@ -419,7 +419,10 @@ complete_address (NMNDisc *ndisc, NMNDiscAddress *addr)
 }
 
 static gboolean
-nm_ndisc_add_address (NMNDisc *ndisc, const NMNDiscAddress *new, gboolean from_ra)
+nm_ndisc_add_address (NMNDisc *ndisc,
+                      const NMNDiscAddress *new,
+                      gint32 now_s,
+                      gboolean from_ra)
 {
 	NMNDiscPrivate *priv = NM_NDISC_GET_PRIVATE (ndisc);
 	NMNDiscDataInternal *rdata = &priv->rdata;
@@ -432,6 +435,7 @@ nm_ndisc_add_address (NMNDisc *ndisc, const NMNDiscAddress *new, gboolean from_r
 	nm_assert (!IN6_IS_ADDR_UNSPECIFIED (&new->address));
 	nm_assert (!IN6_IS_ADDR_LINKLOCAL (&new->address));
 	nm_assert (new->preferred <= new->lifetime);
+	nm_assert (!from_ra || now_s > 0);
 
 	for (i = 0; i < rdata->addresses->len; i++) {
 		NMNDiscAddress *item = &g_array_index (rdata->addresses, NMNDiscAddress, i);
@@ -452,6 +456,51 @@ nm_ndisc_add_address (NMNDisc *ndisc, const NMNDiscAddress *new, gboolean from_r
 	}
 
 	if (existing) {
+		if (from_ra) {
+			const gint32 NM_NDISC_PREFIX_LFT_MIN = 7200; /* seconds, RFC4862 5.5.3.e */
+			gint64 old_expiry_lifetime, old_expiry_preferred;
+
+			old_expiry_lifetime = get_expiry (existing);
+			old_expiry_preferred = get_expiry_preferred (existing);
+
+			if (new->lifetime == NM_NDISC_INFINITY)
+				existing->lifetime = NM_NDISC_INFINITY;
+			else {
+				gint64 new_lifetime, remaining_lifetime;
+
+				/* see RFC4862 5.5.3.e */
+				if (existing->lifetime == NM_NDISC_INFINITY)
+					remaining_lifetime = G_MAXINT64;
+				else
+					remaining_lifetime = ((gint64) existing->timestamp) + ((gint64) existing->lifetime) - ((gint64) now_s);
+				new_lifetime = ((gint64) new->timestamp) + ((gint64) new->lifetime) - ((gint64) now_s);
+
+				if (   new_lifetime > (gint64) NM_NDISC_PREFIX_LFT_MIN
+				    || new_lifetime > remaining_lifetime) {
+					existing->timestamp = now_s;
+					existing->lifetime = CLAMP (new_lifetime, (gint64) 0, (gint64) (G_MAXUINT32 - 1));
+				} else if (remaining_lifetime <= (gint64) NM_NDISC_PREFIX_LFT_MIN) {
+					/* keep the current lifetime. */
+				} else {
+					existing->timestamp = now_s;
+					existing->lifetime = NM_NDISC_PREFIX_LFT_MIN;
+				}
+			}
+
+			if (new->preferred == NM_NDISC_INFINITY) {
+				nm_assert (existing->lifetime == NM_NDISC_INFINITY);
+				existing->preferred = new->preferred;
+			} else {
+				existing->preferred = NM_CLAMP (((gint64) new->timestamp) + ((gint64) new->preferred) - ((gint64) existing->timestamp),
+				                                0, G_MAXUINT32 - 1);
+				if (existing->lifetime != NM_NDISC_INFINITY)
+					existing->preferred = MIN (existing->preferred, existing->lifetime);
+			}
+
+			return    old_expiry_lifetime != get_expiry (existing)
+			       || old_expiry_preferred != get_expiry_preferred (existing);
+		}
+
 		if (new->lifetime == 0) {
 			g_array_remove_index (rdata->addresses, i);
 			return TRUE;
@@ -459,12 +508,10 @@ nm_ndisc_add_address (NMNDisc *ndisc, const NMNDiscAddress *new, gboolean from_r
 
 		if (   get_expiry (existing) == get_expiry (new)
 		    && get_expiry_preferred (existing) == get_expiry_preferred (new)
-		    && (   from_ra
-		        || existing->dad_counter == new->dad_counter))
+		    && existing->dad_counter == new->dad_counter)
 			return FALSE;
 
-		if (!from_ra)
-			existing->dad_counter = new->dad_counter;
+		existing->dad_counter = new->dad_counter;
 		existing->timestamp = new->timestamp;
 		existing->lifetime = new->lifetime;
 		existing->preferred = new->preferred;
@@ -495,9 +542,11 @@ nm_ndisc_add_address (NMNDisc *ndisc, const NMNDiscAddress *new, gboolean from_r
 }
 
 gboolean
-nm_ndisc_complete_and_add_address (NMNDisc *ndisc, const NMNDiscAddress *new)
+nm_ndisc_complete_and_add_address (NMNDisc *ndisc,
+                                   const NMNDiscAddress *new,
+                                   gint32 now_s)
 {
-	return nm_ndisc_add_address (ndisc, new, TRUE);
+	return nm_ndisc_add_address (ndisc, new, now_s, TRUE);
 }
 
 gboolean
@@ -795,7 +844,7 @@ nm_ndisc_set_config (NMNDisc *ndisc,
 	guint i;
 
 	for (i = 0; i < addresses->len; i++) {
-		if (nm_ndisc_add_address (ndisc, &g_array_index (addresses, NMNDiscAddress, i), FALSE))
+		if (nm_ndisc_add_address (ndisc, &g_array_index (addresses, NMNDiscAddress, i), 0, FALSE))
 			changed = TRUE;
 	}
 
diff --git a/src/ndisc/tests/test-ndisc-fake.c b/src/ndisc/tests/test-ndisc-fake.c
index e99d2fc55e..268f3b49b4 100644
--- a/src/ndisc/tests/test-ndisc-fake.c
+++ b/src/ndisc/tests/test-ndisc-fake.c
@@ -233,8 +233,9 @@ test_everything_changed (NMNDisc *ndisc, const NMNDiscData *rdata, guint changed
 
 		g_assert_cmpint (rdata->gateways_n, ==, 1);
 		match_gateway (rdata, 0, "fe80::2", data->timestamp1, 10, NM_ICMPV6_ROUTER_PREF_MEDIUM);
-		g_assert_cmpint (rdata->addresses_n, ==, 1);
-		match_address (rdata, 0, "2001:db8:a:b::1", data->timestamp1, 10, 10);
+		g_assert_cmpint (rdata->addresses_n, ==, 2);
+		match_address (rdata, 0, "2001:db8:a:a::1", data->timestamp1, 10, 0);
+		match_address (rdata, 1, "2001:db8:a:b::1", data->timestamp1, 10, 10);
 		g_assert_cmpint (rdata->routes_n, ==, 1);
 		match_route (rdata, 0, "2001:db8:a:b::", 64, "fe80::2", data->timestamp1, 10, 10);
 		g_assert_cmpint (rdata->dns_servers_n, ==, 1);
@@ -384,7 +385,7 @@ test_preference_changed_cb (NMNDisc *ndisc, const NMNDiscData *rdata, guint chan
 		match_gateway (rdata, 0, "fe80::1", data->timestamp1 + 2, 10, NM_ICMPV6_ROUTER_PREF_HIGH);
 		match_gateway (rdata, 1, "fe80::2", data->timestamp1 + 1, 10, NM_ICMPV6_ROUTER_PREF_MEDIUM);
 		g_assert_cmpint (rdata->addresses_n, ==, 2);
-		match_address (rdata, 0, "2001:db8:a:a::1", data->timestamp1 + 2, 10, 10);
+		match_address (rdata, 0, "2001:db8:a:a::1", data->timestamp1 + 3, 9, 9);
 		match_address (rdata, 1, "2001:db8:a:b::1", data->timestamp1 + 1, 10, 10);
 		g_assert_cmpint (rdata->routes_n, ==, 2);
 		match_route (rdata, 0, "2001:db8:a:a::", 64, "fe80::1", data->timestamp1 + 2, 10, 15);

From 2c47cf51b121635ab7ede44efbc7da5ed087f2e4 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Sat, 13 Oct 2018 17:11:09 +0200
Subject: [PATCH 7/7] ndisc: don't update dad_counter for addresses in router
 config

I am not sure, we ever call complete_address() for router-configurations.
Maybe not, so the dad-counter is never incremented and does not matter either.

If we however do, then we certainly want to preserve the DAD counter
when the address is already tracked.

(cherry picked from commit 8c6629b356039e2b2bbb87574755dad298cb0615)
(cherry picked from commit 036d1f56ea3321056237251cda66e412f7826e7b)
(cherry picked from commit 148c9d9b0cacc4b65a8be6fa66861e06c6847113)
---
 src/ndisc/nm-ndisc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/ndisc/nm-ndisc.c b/src/ndisc/nm-ndisc.c
index 44c8a09d3d..56b82c4cf7 100644
--- a/src/ndisc/nm-ndisc.c
+++ b/src/ndisc/nm-ndisc.c
@@ -507,11 +507,9 @@ nm_ndisc_add_address (NMNDisc *ndisc,
 		}
 
 		if (   get_expiry (existing) == get_expiry (new)
-		    && get_expiry_preferred (existing) == get_expiry_preferred (new)
-		    && existing->dad_counter == new->dad_counter)
+		    && get_expiry_preferred (existing) == get_expiry_preferred (new))
 			return FALSE;
 
-		existing->dad_counter = new->dad_counter;
 		existing->timestamp = new->timestamp;
 		existing->lifetime = new->lifetime;
 		existing->preferred = new->preferred;