Re: kern/52074: -current npf map directive broken

[Frank Kardel <kardel%netbsd.org@localhost>]

Hi Roy and all!

On 05/11/17 11:47, Roy Marples wrote:
> Hi Frank On 10/05/2017 10:11, Frank Kardel wrote:
> > On 05/10/17 00:45, Robert Elz wrote:
> > > Date: Sun, 07 May 2017 23:07:42 +0200 From: Frank Kardel 
> > > <kardel%netbsd.org@localhost> Message-ID: <590F8C9E.3040102%netbsd.org@localhost> | From 
> > > what I understand this code originally attempted to avoid sending | 
> > > from invalid/unusable local address (e. g. duplicate IP - error, | 
> > > tentative and detached should just be dropped). You also shouldn't 
> > > be able to send from an address you don't own (generally - a router 
> > > has to be able to forward, as distinct from originate, packets from 
> > > anywhere of course). 
> > You are correct - in this case (52074) we are looking at both aspects 
> > - the local machine and the router/NAT box. It is *not* about 
> > originating packets from anywhere. It is about redirecting packets 
> > for non local targets to a locally existing proxy. 
> I agree with Robert, we shouldn't be sending packets on the wire from 
> an address we don't own. But you're not sending on the wire are you?
See kre@'s comment. He described it.The local addres gets replaced with 
the remote address we proxied for.
> I think a check to satisfy us all would be to test for IP_FORWARDING 
> on the packet or IFF_LOOPBACK on the outgoing interface - if either 
> are true we can skip address validation. 
The IFF_LOOPBACK property is of no use here. What counts is that the 
redirect is to a local address to masquerade a remote address.
Any local address used for redirect would trigger the issue as the 
packet filter puts back the original remote address and that's violating 
the assumption
the we must use local addresses on locally generated packets.
> Thoughts? Roy
I think the solution is much simpler (as I mentioned before).

If we actually take look at ip_output we see roughly following 
processing steps:
 1) insert ip options
 2) create IP header unless forwarding or raw output
 3) handle routing (choose output interface)
 4) if no source address is set, use the interface address of the 
outgoing interface
 5) handle IPSEC
 6) run packet filter
 7) check whether we are sending from a valid local address (our 
discussion point)
 8) do xmit accounting
 9) handle TSO offloading or packet not needing fragmentation and 
output via interface -> done
10) handle checksum offloading/calculation
11) handle fragmentation output fragmented packets
12) finalize

During 1...5 the assumption/requirement that non IP_FORWARDING packets 
must originate from local addresses holds.
Number 6 breaks this assumption in case of a redirected remote address 
to a local address. There the test in 7 relies
on a requirement that cannot be guaranteed after packet filter NAT 
operations.

Moving ia pickup and !IP_FORWARDING local address check from 7 to 5a 
will achieve what we want (guard against from invalid local addresses). Also
IFA accounting would correctly measure bytes-sent at the right local 
address involved in the redirect scenario.

I am currently running a check on local address violations before the 
packet filter call. The invariant that 1..5 follow the local address 
requirement holds so
far. Traffic processed is: normal connections, ipsec transport/NATT, 
some multicast.

Can somebody verify that strategy as a correct solution?

NB: the current fix has the drawback of at least miscounting output 
bytes and missing some local checks.

Frank