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.

I see this issue now, but something is bugging me about
being forced to use a /31 when a /32 *would* work if the
0x7f000000 was actually 0x7f000001, but that fails for
other cases.

Note oddly the code would work with /32 if I decided to use
127.0.0.0/32 as the IP address on lo0 so should the expression
become:

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

> 
> 		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
> 
> 

-- 
Rod Grimes                                                 rgrimes@freebsd.org