Re: [PATCH nf] netfilter: xt_nat: allow IP_CT_RELATED_REPLY in DNAT targets

[Florian Westphal]

xujiahao <xujiahao@xxxxxxxxxxxxxxxxxxxx> wrote:
> The DNAT targets warn when the conntrack info is neither IP_CT_NEW nor
> IP_CT_RELATED, while the corresponding SNAT targets additionally accept
> IP_CT_RELATED_REPLY:
> 
>   WARN_ON(!(ct != NULL &&
>            (ctinfo == IP_CT_NEW || ctinfo == IP_CT_RELATED)));
> 
> A TCP reset generated by nf_send_reset() on the reject path is attached
> to the conntrack entry of the rejected packet and marked
> IP_CT_RELATED_REPLY.

I don't think thats correct.  nf_conntrack_attach() assumes ICMP.

TCP should just set IP_CT_ESTABLISHED, we're 'emulating' a reset
packet coming from peer, so we should not create a fresh/new nat
rewrite.

I think this should be fixed in nf_conntrack_attach() to check for
TCP and then set IP_CT_ESTABLISHED state for nskb.

> It traverses the LOCAL_OUT NAT hook, where
> nf_nat_inet_fn() lets IP_CT_RELATED_REPLY packets whose destination NAT
> mapping is not initialized yet run the OUTPUT NAT chain.  When that
> chain holds an xtables DNAT target, the target is reached with
> IP_CT_RELATED_REPLY and warns:
> 
>   WARNING: net/netfilter/xt_nat.c:154 at xt_dnat_target_v2+0x97/0xc0

This warning is correct. We should not do a DNAT mapping for
IP_CT_RELATED_REPLY on TCP, it would cause packet rewrite.

The IP_CT_RELATED_REPLY warning is old, from a time when DNAT was
restricted to PRE_ROUTING hooks, which is likely why its only
in the SNAT handler (which was restricted to POST_ROUTING only).