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

[xujiahao <xujiahao@cmss.chinamobile.com>]

Hi Florian,

You are right and my patch was wrong.  I am dropping the xt_nat change; the
check in xt_dnat_target_v2() should stay as it is.

I did reproduce the syzbot report in a netns: a reject rule makes
nf_send_reset() build the RST, which then goes ip_local_out() ->
nf_nat_ipv4_local_fn(), and the RST arrives with ctinfo IP_CT_RELATED_REPLY
(4).  nf_nat_inet_fn() therefore treats it as not initialized yet and runs the
nat chains for it, which is how xt_dnat_target_v2() is reached.  In my test
the conntrack was already confirmed, so nf_nat_setup_info() declined to
install a second mapping and the packet itself was not rewritten -- but the
nat chains should not be consulted for that packet at all.

Before I send a fix for nf_conntrack_attach(): it cannot tell an ICMP error
from a TCP RST on its own.  The icmp.c/icmpv6.c callers run inside a getfrag
callback, before the IP header is built, and the conntrack's L4 protocol is
the protocol of the original packet, which can well be TCP for an ICMP error.
So the packet type has to come from the call site.  Which shape do you prefer?

  a) nf_ct_attach(new, skb, bool tcp_rst) -- changes the exported symbol, or
  b) keep nf_ct_attach() as is and add a separate helper for the reject path?

Either way ICMP keeps IP_CT_RELATED/_REPLY, and only the RST built by
nf_reject_ipv4.c / nf_reject_ipv6.c would get IP_CT_ESTABLISHED/_REPLY.

Thanks,
xujiahao