Subject: Re: CVS commit: syssrc
To: Jun-ichiro itojun Hagino <itojun@netbsd.org>
From: John Hawkinson <jhawk@MIT.EDU>
List: source-changes
Date: 03/01/2001 23:09:41
| Module Name:	syssrc
| Committed By:	itojun
| Date:		Fri Mar  2 02:05:37 UTC 2001
| 
| Modified Files:
| 	  syssrc/sys/netinet: ip_input.c
| 
| Log Message:
| reject packets with 127/8 on IPv4 src/dst, they must not appear on wire
| (RFC1122).  torture-tests will be welcomed.
| XXX do we want to check source routing headers as well?

I am unclear that this is the correct change. I'm not sure
how to state this best, because some of it is in the realm of
"I'm not sure that RFC1122 says that," and some of it is
"even if RFC1122 says that, I'm not sure we should do it."

The principles of simplicity and understandability say to me
that we should not provide special treatment for one IP address
over another, unless there is a compelling reason.

In your code, you say:

+       /* 127/8 must not appear on wire - RFC1122 */

unfortunately, RFC1122 is not so succinct. There are three relevent
pieces, from section 3.2.1.3:

            (g)  { 127, <any> }

                 Internal host loopback address.  Addresses of this form
                 MUST NOT appear outside a host.
...
            A host MUST silently discard an incoming datagram that is
            not destined for the host.  An incoming datagram is destined
            for the host if the datagram's destination address field is:

            (1)  (one of) the host's IP address(es); or

            (2)  an IP broadcast address valid for the connected
                 network; or

            (3)  the address for a multicast group of which the host is
                 a member on the incoming physical interface.
...
            A host MUST silently discard an incoming datagram containing
            an IP source address that is invalid by the rules of this
            section.  This validation could be done in either the IP
            layer or by each protocol in the transport layer.


I don't think that "MUST NOT appear" translates into "we MUST filter
on input."



One of the reasons I am nervous about this is that some people have
been known to use 127/8 addresses for RFC1918-ish purposes. It would
be unfortunate if that broke for no particularly good reason.

It can certainly be argued that 127/8 addresses are "not one of the host's
IP addresses", but that doesn't _necessarily_ apply to 127.0.0.1, which
can be construed as one of the host's addresses.


It seems to me that there should be a counter for assocaited with rejecting
these packets, if they are going to have a special case block of code
(as they do in your patch). If you don't think so, I'd like to see the
argument against.



I feel like we're treating 127/8 specially for a reason that is not
particularly good, though. It's not as if there is an important
security issue here -- applications should not have been depending on
127.0.0.1 as being a local IP address, and I am unclear on how this
patch makes that sort of thing more secure, since we can still receive
packets that are sourced from an IP address associated with any of our
interface that are not 127.0.0.1. So I do not see how we get a
security benefit.


As such, from the releng-1-5 perspective, I'm disinclined to accept
your pullup request -- but that's a different issue.

D'you think we should move this to tech-net, or will we finish it here?

--jhawk