NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

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

On 05/08/17 11:04, Roy Marples wrote:
Hi Frank!

On 07/05/2017 22:07, Frank Kardel wrote:
Hmm, wouldn't this bring us back the bug again? ia == NULL for a
non-local source addresses (generated via pfil_run_hooks-NAT operation)
and IP_FORWARDING is not set as tcp_input.c:syn_cache_respond does
rightfully not set IP_FORWARDING and pfil_run_hooks has no means to set
that flag. That gives us error == -1 with your sequence.
So we would return EADDRNOTAVAIL breaking packet filter NAT action
again, if I didn't overlook something.
Ah yes.
I didn't actually read your initial report .... my bad!
No worries - I am just trying to keep us from running too far in the wrong direction.
OK, so I don't fully understand the use case for sending packets from an
address we don't have locally. Could you fill me in on this please?
The mapping is as follows

Machine A (Client) Machine B (Gateway/NAT/redirect box) Machine C (target) a.b.c.d:x -> W.X.Y.Z -> (will never see a packet from a.b.c.dx)
                          map 25 <- 25
                          redirects to 25 (local)
A must see the association a.b.c.d:x <->

Thus local postfix will see a.b.c.d x <-> 25

So dest addr will be replaced incoming with 25 in pf_fil_run_hooks in ip_input and on the way back the src addr of is replaced with (not local)
                          in pfil_run_hooks in ip_output.

To quote the condensed analysis from the bug + where in the stack it happens:

 Following happens:
 1) ip_input: packet arrives on interface (a.b.c.d:x ->
 1a) ip_input call pfil_run_hooks -> NPF
 2) NPF: NAT assoc gets created
 3) NPF: packet gets NATed (=> a.b.c.d:x ->
 3a) NPF returns
 3b) ip_input hands over to tcp_input
 4) tcp_input: packet gets entered in syn_cache
 5) tcp_input: syn_cache attempts to send SYN,ACK response ( ->
 a.b.c.d:x) via ip_output()
 6) ip_output: ip_output calls pfil_hooks (->NPF)
 7) NPF: find nat assoc
 8) NPF: SYN,ACK packet gets reverse NATed ( -> a.b.c.d:x)
 8a) NPF returns
 9) ip_output: ip_output tries for find the interface adress (ia) for and
 fails (ia == NULL)
 10) ip_output: ip_output fumbles badly at ip_output:1.276:620 and declares the
 packet coming from an invalid address(ip_ifaddrvalid(NULL))
 11) confusion commences from here on

 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).
No validation can be done for non-local addresses at all. IP_FORWARDING
formerly used to be used to suppress infinite recursion on mcast
forwarding, but it seems the semantics where extended a little bit in
the mean time (like here to suppress a check).
So I cannot say something about the intentions for the IP_FORWARDING check.
I *think* the idea was IP_FORWARDING would be set and we could skip
source address validation because the filter may have changed it.
That's what I thought was the intention. As you see above the transmitted SYN,ACK is not created by NPF and not by npf_sendpkt.c#178. NPF and other filters do an in place header modification and the flags variable is not part of pfil_run_hooks(). So
the IP_FORWARDING logic cannot be used by the packet filters AFAICS.
Does NAT not hit that?
Nope - see above - coming directly from tcp_input:syn_cache_add:syn_cache_respond. IP_FORWARD is not set there. Before the packet filter we still have a valid local address, Maybe we should move the check there - IPv6 also does the check before IIANM.
  I ask because I do run NPF+NAT on my erlite
router which uses -current, but my mapping rule uses a local address -
hence asking for your use case about a non local address.

Could NAT set another flag we could check? rmind?
1) extend pfil_run_hooks interface to pass a pointer to flags- relatively cheap, somewhat dirty 2) add a tag the the mbuf - more expensive b/c we also need to add checks

Would we gain enough speed/functionality/abstraction to warrant an interface change? Does the condition "before packet filter we have either IP_FORWARD set or a local address as source" always hold? If so, we could just move the check and set up ia correctly for IFASTATS.IPv6 checks before packet filter runs.

I have not analyzed the invariants enough to make a quick recommendation here.

I hope I explained the setup good enough to describe the issue with NAT redirection.


Home | Main Index | Thread Index | Old Index