On Mon, Jun 11 2018, Klemens Nanni <k...@openbsd.org> wrote:
> Here's a new diff that removes the duplicate parsing bits as mentioned
> before but leaves masking the address to mask_addr() instead of doing it
> manually.
>
> Furthermore, it also stops route(8) from assuming address strings
> without explicit prefix length to be /64.
>
> The old behaviour described in RFC 2374 from 1998 is obsolete as per
> RFC 3587 which states
>
>       RFC 2374 was the definition of addresses for Format Prefix 001
>       (2000::/3) which is formally made historic by this document.  Even
>       though currently only 2000::/3 is being delegated by the IANA,
>       implementations should not make any assumptions about 2000::/3 being
>       special.  In the future, the IANA might be directed to delegate
>       currently unassigned portions of the IPv6 address space for the
>       purpose of Global Unicast as well.
>
> This was brought to my attention by the (recent) misc@ thread
> "Confusing IPv6 route(8) results"[0] in which benno@ mentioned an
> important side effect when it comes to using `-prefixlen' before the
> address string since order is relavant here and the following would
> therefore blackhole a single /128 with after my patch applied:
>
>       # route add -inet6 -prefixlen 56 2001:db8:: ::1 -blackhole
>
> Thus, I'll put a note into current.html in case this change should go in.
>
> FWIW, both FreeBSD and NetBSD already use `prefixlen()' and do not mask
> the address in `inet6_makenetandmask()'. NetBSD still assumes /64 as per
> RFC 2374 while FreeBSD does the same already I'm aiming for.
>
> Regress passes, no regressions found when manually adding/removing
> routes.
>
> Feedback? Objections? OK?

Sorry for not caring about this sooner.  Probably because it looked like
a can of worms...

So if I understand correctly, this diff does three things:
1. shorten the code (remove the explicit mask handling from this
   function)
2. stop considering 2000::/3 as special per RFC3587
3. stop considering 2001:db8:: as a /64 prefix by default, but consider
   it as a host (/128)

1. might be desirable, even though I find it hard to follow the code
   flow.  Perhaps the code should be more explicit and stop abusing
   globals...

2. fine, but...

3. ... why should we stop assuming that the user really means to
   configure a route for a /64 if the host id part is all-zeroes?  Is
   this really part of what has been deprecated by RFC3587?

I can understand that having the same behavior (host address is no
prefix length is specified) with v4 and v6 is desirable, but as benno
pointed out, some people might be relying on the current default
behavior.  That's not a strong objection, but I'd like to know what's
the exact rationale behind this change.

An alternate way of fixing item 2 would be to keep the current behavior
but extend it to any foo:bar:: address, not just to addresses within
2000::/3.


Index: route.c
===================================================================
RCS file: /d/cvs/src/sbin/route/route.c,v
retrieving revision 1.215
diff -u -p -p -u -r1.215 route.c
--- route.c     18 Jun 2018 09:17:06 -0000      1.215
+++ route.c     24 Jun 2018 14:31:46 -0000
@@ -792,7 +792,7 @@ inet_makenetandmask(u_int32_t net, struc
 int
 inet6_makenetandmask(struct sockaddr_in6 *sin6, char *plen)
 {
-       struct in6_addr in6;
+       static const struct in6_addr zero_in6;
        const char *errstr;
        int i, len, q, r;
 
@@ -800,12 +800,9 @@ inet6_makenetandmask(struct sockaddr_in6
                if (IN6_IS_ADDR_UNSPECIFIED(&sin6->sin6_addr) &&
                    sin6->sin6_scope_id == 0) {
                        plen = "0";
-               } else if ((sin6->sin6_addr.s6_addr[0] & 0xe0) == 0x20) {
-                       /* aggregatable global unicast - RFC2374 */
-                       memset(&in6, 0, sizeof(in6));
-                       if (!memcmp(&sin6->sin6_addr.s6_addr[8],
-                           &in6.s6_addr[8], 8))
-                               plen = "64";
+               } else if (!memcmp(&sin6->sin6_addr.s6_addr[8],
+                   &zero_in6.s6_addr[8], 8)) {
+                       plen = "64";
                }
        }
 

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Reply via email to