Fix rt_cmp_dest() for equivalent network prefixes with different netmasks. (#52)

When rt_add() decides that it must delete+add a route in order to change
the routing table entry, a wrong RB tree lookup result can throw it off
the rails. In the case observed, a static /64 prefix was deleted from
vlan1 while dhcpcd intended to delete its reject route bound to lo0.

Given two routes in the table, the loopback reject route installed by
dhcpd for my /48 prefix, and a cloning route for a /64 prefix on vlan1:

2001:db8::/48 ::1         UGR        0        0 32768    56 lo0
2001:db8::/64 2001:db8::1 UCn        1        2     -     4 vlan1

When searching the OS routing table dhcpcd attempts to tell routes apart
based only on the masked destination address. In the above case the masked
destinations look identical. The only difference is the length of the netmask.
The function rt_cmp_dest() didn't detect this and returned the /64 route
while dhcpcd was in fact searching for the /48 route.

This patch fixes the lookup by running rt_cmp_netmask() if the masked
destination comparison via sa_cmp() leaves us with a tie. With this change
dhcpcd deletes the /48 route as intended, and leaves the /64 route alone.

I had to move the rt_cmp_dest() function down since it needs to use the
static helper function rt_cmp_netmask(), which happened to be defined
just below rt_cmp_dest().

Why am I using an overlapping static prefix? The answer is that my ISP
assigns a static /48 prefix but won't route IPv6 unless my router sends
a DHCPv6 request when it connects via PPPoE. I configure static IPv6 subnets
on LAN interfaces and have configured dhcpcd to obtain a /48 prefix lease
without setting addresses on any internal interfaces.
My dhcpcd.conf contains:
  ipv6only
  noipv6rs
  duid
  persistent
  option rapid_commit
  require dhcp_server_identifier
  script ""
  allowinterfaces pppoe0
  interface pppoe0
    ia_pd 1 /2001:db8::/48

This problem was found on OpenBSD, in case that matters for reproduction
of the issue.

diff --git a/src/route.c b/src/route.c
index a5ec517ba3630022e65d58bd8dee9f088a465858..ef9c412580001a761c7ecad54e825a4e58f80c17 100644 (file)
--- a/src/route.c
+++ b/src/route.c
@@ -96,17 +96,6 @@ rt_maskedaddr(struct sockaddr *dst,
                memset(dstp, 0, (size_t)(addre - dstp));
 }
 
-int
-rt_cmp_dest(const struct rt *rt1, const struct rt *rt2)
-{
-       union sa_ss ma1 = { .sa.sa_family = AF_UNSPEC };
-       union sa_ss ma2 = { .sa.sa_family = AF_UNSPEC };
-
-       rt_maskedaddr(&ma1.sa, &rt1->rt_dest, &rt1->rt_netmask);
-       rt_maskedaddr(&ma2.sa, &rt2->rt_dest, &rt2->rt_netmask);
-       return sa_cmp(&ma1.sa, &ma2.sa);
-}
-
 /*
  * On some systems, host routes have no need for a netmask.
  * However DHCP specifies host routes using an all-ones netmask.
@@ -122,6 +111,22 @@ rt_cmp_netmask(const struct rt *rt1, const struct rt *rt2)
        return sa_cmp(&rt1->rt_netmask, &rt2->rt_netmask);
 }
 
+int
+rt_cmp_dest(const struct rt *rt1, const struct rt *rt2)
+{
+       union sa_ss ma1 = { .sa.sa_family = AF_UNSPEC };
+       union sa_ss ma2 = { .sa.sa_family = AF_UNSPEC };
+       int c;
+
+       rt_maskedaddr(&ma1.sa, &rt1->rt_dest, &rt1->rt_netmask);
+       rt_maskedaddr(&ma2.sa, &rt2->rt_dest, &rt2->rt_netmask);
+       c = sa_cmp(&ma1.sa, &ma2.sa);
+       if (c != 0)
+               return c;
+
+       return rt_cmp_netmask(rt1, rt2);
+}
+
 static int
 rt_compare_os(__unused void *context, const void *node1, const void *node2)
 {