Bug 258874 - route add -inet 240/4 results in 0.0.0.0/4 127.0.0.1 UGRS lo0
Summary: route add -inet 240/4 results in 0.0.0.0/4 127.0.0.1 UGRS lo0
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 13.0-RELEASE
Hardware: amd64 Any
: --- Affects Some People
Assignee: Mike Karels
URL:
Keywords: needs-qa
: 276092 (view as bug list)
Depends on:
Blocks:
 
Reported: 2021-10-02 22:00 UTC by Jason Mader
Modified: 2024-01-22 17:05 UTC (History)
7 users (show)

See Also:
karels: mfc-stable14+
karels: mfc-stable13+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jason Mader 2021-10-02 22:00:05 UTC
On FreeBSD 13.0

# route add -inet 240/4 localhost -reject
add net 240: gateway localhost

results in:

Internet:
Destination        Gateway            Flags     Netif Expire
0.0.0.0/4          127.0.0.1          UGRS        lo0

# route add -inet 224/24 localhost -reject
add net 224: gateway localhost

Internet:
Destination        Gateway            Flags     Netif Expire
0.0.0.0/24         127.0.0.1          UGRS        lo0
Comment 1 Marek Zarychta 2021-10-03 06:35:14 UTC
I am not able to reproduce this on recent stable/13. The commands:
route add -net 240.0.0.0/4 localhost -[reject|blackhole] 
work as expected.
Please upgrade to STABLE if possible.
Comment 2 Jason Mader 2021-10-03 17:22:32 UTC
(In reply to Marek Zarychta from comment #1)

You added .0.0.0 to the command, and that command does work as expected. The command I reported does not work as expected in 13.0-RELEASE-p4.
Comment 3 Jason Mader 2021-10-03 17:34:14 UTC
(In reply to Marek Zarychta from comment #1)

I booted FreeBSD-13.0-STABLE-amd64-20210930 and tried the route command and it did not work as I expected, which is that -inet 224/4 would install the route for 224.0.0.0/4 and not 0.0.0.0/4.
Comment 4 Mike Karels freebsd_committer freebsd_triage 2021-10-03 18:01:34 UTC
According to inet(3), an Internet address specified as a single component without dots is not shifted; i.e. 224 is the same as 0.0.0.224.  I would not expect this command to work as you expect it to.
Comment 5 Marek Zarychta 2021-10-03 18:28:40 UTC
(In reply to Mike Karels from comment #4)
I was expecting the same result (0.0.0.0.0/4, 240 == 0.0.0.240 != 240.0.0.0),
but I have checked and it came out that for FreeBSD 12 and earlier 240 is 240.0.0.0

Test:
route add -net 240/32 localhost -blackhole 
netstat -4rn |grep 240

FreeBSD 12.2 and earlier:
240.0.0.0/32       127.0.0.1          UGSB        lo0

FreeBSD 13.0 and later:
0.0.0.240          127.0.0.1          UGHSB       lo0

Linux and FreeBSD 13.0 network stack work the same in this case and IMHO as expected. Was the old behavior introduced due to any reasons or was it a bug?
Comment 6 Alexander V. Chernikov freebsd_committer freebsd_triage 2021-10-03 21:35:21 UTC
For the record, the previous behaviour was provided by the following code in route.c:

```
	/*
	 * MSB of net should be meaningful. 0/0 is exception.
	 */
	if (net > 0)
		while ((net & 0xff000000) == 0)
			net <<= 8;
```

https://cgit.freebsd.org/src/tree/sbin/route/route.c?h=stable/12#n1123


This part was removed as a classful bits cleanup here: https://cgit.freebsd.org/src/commit/sbin/route/route.c?id=d28210b2c2aaf3200907ed30d296b0d4856dd03c


I have mixed opinions on that.
Using 10/8 or 240/4 is certainly convenient.
However, it effectively goes again the behaviour specified in inet(3), so it may be a bit confusing for the people w/o muscular memory on remembering that worked.
I'd rather prefer to leave it in a current state.
Comment 7 Alexander V. Chernikov freebsd_committer freebsd_triage 2021-10-23 09:47:25 UTC
I'm going to close it with 'work as expected' on November 1 if there are no other objections.
Comment 8 Mike Karels freebsd_committer freebsd_triage 2021-10-23 15:39:16 UTC
I also have mixed feelings about this.  I dislike the code that was removed.  But the fact that 240/4 or 10/8 used to mean something useful and now mean something not useful suggests that the quoted bit of code should be restored.  Another option might be to warn if the pattern specified has bits not under the mask (i.e. in the "host part").  But I think the best option is to restore the quoted code.
Comment 9 Mike Karels freebsd_committer freebsd_triage 2024-01-07 14:28:32 UTC
I have a change to route to detect this error and complain about it.  In response to "route add 10/8 ip", it prints this and exits non-zero:

route: malformed address, bits set after mask; 10 means 0.0.0.10

Comments?  I'll put the change into review soon.
Comment 10 paul vixie 2024-01-07 17:52:40 UTC
in inet_addr(3) we see this text:

   INTERNET ADDRESSES
     Values specified using the ‘.’ notation take one of the following forms:

           a.b.c.d
           a.b.c
           a.b
           a

     When four parts are specified, each is interpreted as a byte of data and
     assigned, from left to right, to the four bytes of an Internet address.
     Note that when an Internet address is viewed as a 32-bit integer quantity
     on the VAX the bytes referred to above appear as “d.c.b.a”.  That is, VAX
     bytes are ordered from right to left.

     When a three part address is specified, the last part is interpreted as a
     16-bit quantity and placed in the least significant two bytes of the
     network address.

     When a two part address is supplied, the last part is interpreted as a
     24-bit quantity and placed in the least significant three bytes of the
     network address.

     When only one part is given, the value is stored directly in the network
     address without any byte rearrangement.

this is echoed by inet_net_ntop(3) -- same text.

so, i never liked this or understood it, and i did not preserve this syntax in inet_pton(), but it's still documented, and i think we ought to either make sure it isn't documented and never works anywhere, or that "route" should support it.

i did preserve this in inet_net_pton(), so if "route" were to just use that, then these examples (10/8 or even just 10) would work as seems to still be expected. separately, inet_network() probably should use inet_net_pton() now.
Comment 11 Mike Karels freebsd_committer freebsd_triage 2024-01-07 19:21:02 UTC
(In reply to paul vixie from comment #10)
I hadn't realized that inet_pton required 4 components; it took a search in the man page to find that in the standards section.  The man page should be clarified, in particular the INTERNET ADDRESSES section, to say which functions use these conventions.

> so, i never liked this or understood it, and i did not preserve this syntax in inet_pton(), but it's still documented, and i think we ought to either make sure it isn't documented and never works anywhere, or that "route" should support it.

It's far too late to change inet_aton(), etc; they have had this behavior for 40 years, and it is in the other BSDs too.  In fact, route uses inet_aton(), and supports this behavior.  It just isn't the behavior that some people expect.  Worse, as noted in comment 6, route had special code to "do what I mean" and shifted the address until the most-significant octet was non-zero.  That was removed from route about 13.x.  When this bug was first opened in 2021, I thought that we should restore that behavior to avoid breaking things.  However, it has been long enough that most people have adapted.  Now I think we should detect the incorrect values and fail, rather than doing something nonsensical.  Unfortunately, we had neither change in place for 13.2 or 14.0, and now there are new people upgrading from 12.x now that it is EOL.
Comment 12 Ed Maste freebsd_committer freebsd_triage 2024-01-08 16:49:07 UTC
> I have a change to route to detect this error and complain about it.

One other thing we could consider: restoring the special case from comment #6 but also emit a warning when we encounter it, in 13.x and 14.x. In 15.x make it an error.

But that may be more than necessary, given that we've been in this current state for a while.
Comment 13 Mike Karels freebsd_committer freebsd_triage 2024-01-09 21:17:42 UTC
See https://reviews.freebsd.org/D43384.
Comment 14 commit-hook freebsd_committer freebsd_triage 2024-01-15 21:16:11 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=b9e8ae1d8a424194b4e185359da4ded163f24f4e

commit b9e8ae1d8a424194b4e185359da4ded163f24f4e
Author:     Mike Karels <karels@FreeBSD.org>
AuthorDate: 2024-01-15 21:14:54 +0000
Commit:     Mike Karels <karels@FreeBSD.org>
CommitDate: 2024-01-15 21:14:54 +0000

    route: error on IPv4 network routes with incorrect destination

    Route destinations like 10/8 are most likely intended as a shorthand
    for 10.0.0.0/8, but instead it means 0.0.0.10/8, which includes
    only bits in the host part of the mask, and hence adds a route to
    0.0.0.0/8.  In 12.x, there was code to "do what I mean", which was
    removed as part of a cleanup of old network class remnants.  Given
    that we have gone this long without that code, do not restore that
    behavior.  Instead, detect the issue and produce an error.
    Specifically, if there are no dots in a numeric IPv4 address, the
    mask is specified with CIDR notation (using a slash), and there are
    bits set in the host part, produce an error like this for 10/8:

        route: malformed address, bits set after mask; 10 means 0.0.0.10

    PR:             258874
    MFC after:      1 week
    Reviewed by:    melifaro, emaste
    Differential Revision:  https://reviews.freebsd.org/D43384

 sbin/route/route.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
Comment 15 Mike Karels freebsd_committer freebsd_triage 2024-01-15 22:59:35 UTC
I plan to close this bug as Fixed unless someone objects.  The change that was committed treats this as an error, so it is not the same as in 12.4.  But it won't create bogus routes and should clarify the problem.
Comment 16 Mike Karels freebsd_committer freebsd_triage 2024-01-15 23:16:29 UTC
I should also mention that this change will be MFC'd to stable/14 and stable/13.
Comment 17 commit-hook freebsd_committer freebsd_triage 2024-01-22 16:43:32 UTC
A commit in branch stable/14 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=7e88d8fec4e8adc258378c7a68adf6cef1da8ad4

commit 7e88d8fec4e8adc258378c7a68adf6cef1da8ad4
Author:     Mike Karels <karels@FreeBSD.org>
AuthorDate: 2024-01-15 21:14:54 +0000
Commit:     Mike Karels <karels@FreeBSD.org>
CommitDate: 2024-01-22 16:42:16 +0000

    route: error on IPv4 network routes with incorrect destination

    Route destinations like 10/8 are most likely intended as a shorthand
    for 10.0.0.0/8, but instead it means 0.0.0.10/8, which includes
    only bits in the host part of the mask, and hence adds a route to
    0.0.0.0/8.  In 12.x, there was code to "do what I mean", which was
    removed as part of a cleanup of old network class remnants.  Given
    that we have gone this long without that code, do not restore that
    behavior.  Instead, detect the issue and produce an error.
    Specifically, if there are no dots in a numeric IPv4 address, the
    mask is specified with CIDR notation (using a slash), and there are
    bits set in the host part, produce an error like this for 10/8:

        route: malformed address, bits set after mask; 10 means 0.0.0.10

    PR:             258874
    Reviewed by:    melifaro, emaste
    Differential Revision:  https://reviews.freebsd.org/D43384

    (cherry picked from commit b9e8ae1d8a424194b4e185359da4ded163f24f4e)

 sbin/route/route.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
Comment 18 commit-hook freebsd_committer freebsd_triage 2024-01-22 16:43:34 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=74e52718aa737deb2477350b16697def98259836

commit 74e52718aa737deb2477350b16697def98259836
Author:     Mike Karels <karels@FreeBSD.org>
AuthorDate: 2024-01-15 21:14:54 +0000
Commit:     Mike Karels <karels@FreeBSD.org>
CommitDate: 2024-01-22 16:42:52 +0000

    route: error on IPv4 network routes with incorrect destination

    Route destinations like 10/8 are most likely intended as a shorthand
    for 10.0.0.0/8, but instead it means 0.0.0.10/8, which includes
    only bits in the host part of the mask, and hence adds a route to
    0.0.0.0/8.  In 12.x, there was code to "do what I mean", which was
    removed as part of a cleanup of old network class remnants.  Given
    that we have gone this long without that code, do not restore that
    behavior.  Instead, detect the issue and produce an error.
    Specifically, if there are no dots in a numeric IPv4 address, the
    mask is specified with CIDR notation (using a slash), and there are
    bits set in the host part, produce an error like this for 10/8:

        route: malformed address, bits set after mask; 10 means 0.0.0.10

    PR:             258874
    Reviewed by:    melifaro, emaste
    Differential Revision:  https://reviews.freebsd.org/D43384

    (cherry picked from commit b9e8ae1d8a424194b4e185359da4ded163f24f4e)

 sbin/route/route.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
Comment 19 Mike Karels freebsd_committer freebsd_triage 2024-01-22 16:52:53 UTC
Fixed by preventing bogus routes from being added, and explaining the problem in the error message.  MFC'd to stable/14 and stable/13.
Comment 20 Mike Karels freebsd_committer freebsd_triage 2024-01-22 16:55:03 UTC
*** Bug 276092 has been marked as a duplicate of this bug. ***