Hi,
On Mon, 29 Jun 2020 10:12:23 +0200
Martin Pieuchot <m...@openbsd.org> wrote:
> On 28/06/20(Sun) 20:41, YASUOKA Masahiko wrote:
>> Hi,
>>
>> When "::/0" is used as "default",
>>
>> # route add ::/0 fe80::1%em0
>> add net ::/0: gateway fe80::1%em0: Invalid argument
>>
>> route command trims the sockaddr to { .len = 2, .family = AF_INET6 }
>> for "::/0", but rtable_satoplen() refuses it. I think it should be
>> accepted.
>
> rtable_satoplen() is used in many places, not just in the socket parsing
> code used by route(8). I don't know what side effects can be introduced
> by this change.
>
> Why is IPv6 different from IPv4 when it comes to the default route?
Diferent functions are used. route(8) uses inet_makenetandmask() to
create a sockaddr for IPv4 prefix length and uses prefixlen() for IPv6
prefix length. "/0" results:
IPv4
{ .len = 1, .family = 0, ... }
IPv6
{ .len = 2, .family = AF_INET6, ... }
Next, route(8) uses mask_addr() to trim the tailing zeros.
1129 void
1130 mask_addr(union sockunion *addr, union sockunion *mask, int which)
1131 {
1132 int olen = mask->sa.sa_len;
1133 char *cp1 = olen + (char *)mask, *cp2;
1134
1135 for (mask->sa.sa_len = 0; cp1 > (char *)mask; )
1136 if (*--cp1 != '\0') {
1137 mask->sa.sa_len = 1 + cp1 - (char *)mask;
1138 break;
1139 }
See #1135 carefully. As the results, the sockaddrs become:
IPv4
{ .len = 0, .family = 0, ... }
IPv6
{ .len = 2, .family = AF_INET6, ... }
Yes, we can fix IPv6 case to have .len = 0 as well.
But I thought kernel should accept both cases, since the
representation for IPv6 didn't seem so bad for me.
> Shouldn't we change route(8) to have a `sa_len' of 0?
>
> That would make the following true:
>
> mlen = mask->sa_len;
>
> /* Default route */
> if (mlen == 0)
> return (0)
>
>> Allow sockaddr for prefix length be trimmed before the key(address)
>> field. Actually "route" command trims at the address family field for
>> "::/0"
>>
>> Index: sys/net/rtable.c
>> ===================================================================
>> RCS file: /cvs/src/sys/net/rtable.c,v
>> retrieving revision 1.69
>> diff -u -p -r1.69 rtable.c
>> --- sys/net/rtable.c 21 Jun 2019 17:11:42 -0000 1.69
>> +++ sys/net/rtable.c 28 Jun 2020 11:30:54 -0000
>> @@ -887,8 +887,8 @@ rtable_satoplen(sa_family_t af, struct s
>>
>> ap = (uint8_t *)((uint8_t *)mask) + dp->dom_rtoffset;
>> ep = (uint8_t *)((uint8_t *)mask) + mlen;
>> - if (ap > ep)
>> - return (-1);
>> + if (ap >= ep)
>> + return (0);
>
> That means the kernel now silently ignore sockaddr short `sa_len'. Are
> they supposed to be supported or are they symptoms of bugs?
I have missed rtable_satoplen() is used by other functions.
>
>> /* Trim trailing zeroes. */
>> while (ap < ep && ep[-1] == 0)
>
