ICMP packets associated with NAT connections sent out wrong interface?

[Patrick McHardy]

On Sat, 7 Jul 2007, Yasuyuki KOZAKAI wrote:

> OK, I try to explain what the current netfilter does in Jordan's situation.
> I also attach the patch to fix this problem. Jordan, can you try it ?
>
> Patrick, please consider to apply it. Even if following my idea is not
> correct, clearly it fixes a bug. I'm wondering why I missed '!'
> when I copied & pasted ip_conntrack_proto_icmp.c...


I'm currently travelling, I'll look into it tomorrow. Just one
question below ..

> My idea is following.
>
> - This TCP reset is not initial packet of TCP connection. If it is initial packet,
>  no address in ICMP packet should be mangled. Jordan, if you see
>  /proc/net/netfilter/nf_conntrack, you will find the entry matched the TCP packet.
>
> - TCP packet was marked as error packet. Because '--state ESTABLISHED'
>  didn't match the packet. No conntrack entry wasn't assigned to the
>  packet. Usually, error log by nf_conntrack_tcp should be generated in
>  such case, but no message is generated in some cases. I don't know why
>  this TCP reset was handled as error.
>
> - Then ICMP error generated at this router was not assigned to any conntrack entry.
>
> - nf_conntarck_icmp.c assigns the ICMP error to the conntrack which matches
>  the TCP reset. But IP_CT_IS_REPLY didn't set to *ctinfo. This is bug.
>
>        h = nf_conntrack_find_get(&innertuple, NULL);
>        if (!h) {
>                /* Locally generated ICMPs will match inverted if they
>                   haven't been SNAT'ed yet */
>                /* FIXME: NAT code has to handle half-done double NAT --RR  */
>                if (hooknum == NF_IP_LOCAL_OUT)
>                        h = nf_conntrack_find_get(&origtuple, NULL);


The local ICMP tracking is basically useless nowadays since we always
manually attach the conntrack reference from the original packet
(exactly because of the half-done double NAT FIXME quoted above).
But this is an interesting case, the connection tracking code itself
thought the RST was invalid, but ICMP tracking will still associate
the ICMP containing the RST with the original connection. I'm wondering
whether it should really do that. In case it shouldn't, just removing
all locally generated ICMP special-casing should also fix the bug,
right?