Re: git: fb8ef16bab0d - main - IPv4: correct limit on loopback_prefix

On 21 Jul 2022, at 11:21, Rodney W. Grimes wrote:

>> The branch main has been updated by karels:
>>
>> URL: https://cgit.FreeBSD.org/src/commit/?id=fb8ef16bab0d23e185deed5a6b2e44e72ad53d43
>>
>> commit fb8ef16bab0d23e185deed5a6b2e44e72ad53d43
>> Author:     Mike Karels <karels@FreeBSD.org>
>> AuthorDate: 2022-07-21 13:10:15 +0000
>> Commit:     Mike Karels <karels@FreeBSD.org>
>> CommitDate: 2022-07-21 14:38:17 +0000
>>
>>     IPv4: correct limit on loopback_prefix
>>
>>     Commit efe58855f3ea allowed the net.inet.ip.loopback_prefix value
>>     to be 32.  However, with a 32-bit mask, 127.0.0.1 is not included
>>     in the reserved loopback range, which should not be allowed.
>>     Change the max prefix length to 31.
>
> Hummm... 127.0.0.1/32 specifices exactly and ONLY 127.0.0.1, and
> this should be fine.  Looking at the mask calculated below with
> loopback_prefix=32 this should yeild a mask of 0xffffffff, which
> appears to be exactly what is correct.  What DOES become an issue
> when /32 is used is that the loopback ROUTE 127.0.0.0/32 is wrong
> now, but then with a /32 you dont need a network route, as you
> should have a host route to exactly 127.0.0.1.
>
> Can you be more descriptive on what problem arrose with /32?

You are thinking about this the way I did originally; but the mask
doesn’t apply to 127.0.0.1 directly.  The test is

#define IN_LOOPBACK(i) \
    (((in_addr_t)(i) & V_in_loopback_mask) == 0x7f000000)

So if considering whether to forward 127.0.0.1, we’ll incorrectly
say it’s OK if the prefixlen is 32 (mask of 255.255.255.255).
In that case, only 127.0.0.0 is considered loopback.

John Gilmore pointed out the problem.

		Mike
>> ---
>>  sys/netinet/in.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/sys/netinet/in.c b/sys/netinet/in.c
>> index c3880c4ba983..1c44623bdec1 100644
>> --- a/sys/netinet/in.c
>> +++ b/sys/netinet/in.c
>> @@ -297,7 +297,7 @@ sysctl_loopback_prefixlen(SYSCTL_HANDLER_ARGS)
>>  	error = sysctl_handle_int(oidp, &preflen, 0, req);
>>  	if (error || !req->newptr)
>>  		return (error);
>> -	if (preflen < 8 || preflen > 32)
>> +	if (preflen < 8 || preflen > 31)
>>  		return (EINVAL);
>>  	V_in_loopback_mask = 0xffffffff << (32 - preflen);
>>  	return (0);
>>
>
> -- 
> Rod Grimes                                                 rgrimes@freebsd.org