On Fri, 13 Sept 2024 at 21:35, Phil Sutter <phil@xxxxxx> wrote:
>
> Hi Antonio,
>
> On Fri, Sep 13, 2024 at 05:38:21PM +0200, Antonio Ojea wrote:
> > On Fri, 13 Sept 2024 at 16:18, Florian Westphal <fw@xxxxxxxxx> wrote:
> > >
> > > Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> > > > > Hmm. Looking at nft_nat.c, 'accept' seems consistent with what nat
> > > > > statements do implicitly.
> > > >
> > > > Yes, and xt_TPROXY does NF_ACCEPT.
> > > >
> > > > On the other hand, I can see it does NF_DROP it socket is not
> > > > transparent, it does NFT_BREAK instead, so policy keeps evaluating the
> > > > packet.
> > >
> > > Yes, this is more flexible since you can log+drop for instance in next
> > > rule(s) to alert that proxy isn't running for example.
> > >
> >
> > This is super useful, in the scenario that the transparent proxy takes
> > over the DNATed virtual IP, if the transparent proxy process is not
> > running the packet goes to the DNATed virtual IP so the clients don't
> > observe any disruption.
>
> So here's a use-case for why non-terminal tproxy statement is superior
> over terminal one. :)
>
> > > > > > is this sufficient in your opinion? If so, I made this quick update
> > > > > > for man nft(8).
> > > > >
> > > > > Acked-by: Phil Sutter <phil@xxxxxx>
> > > > >
> > > > > In addition to that, I will update tproxy_tg_xlate() in iptables.git to
> > > > > emit a verdict, too.
> > > >
> > > > Thanks, this is very convenient.
> > >
> > > Agreed, it should append accept keyword in the translator.
> >
> > I'm not completely following the technical details sorry.
>
> In essence, tproxy statement does not set a verdict unless it fails to
> find a suitable socket. A sample ruleset illustrating this is:
>
> | table t {
> | chain c {
> | type filter hook prerouting priority 0
> | tproxy to :1234 log "packet tproxied" accept
> | log "no socket on port 1234 or not transparent?" drop
> | }
> | }
>
> > With my current configuration I do set an accept action
> >
> > udp dport 53 tproxy ip to 127.0.0.1:46659 accept
> >
> > and I have dnat statements after that action.
>
> For the record, your ruleset looks like this (quoting from the kselftest
> you sent me):
>
> | table inet filter {
> | chain divert {
> | type filter hook prerouting priority -100; policy accept;
> | $ip_proto daddr $virtual_ip udp dport 8080 tproxy ip_proto to :12345 meta mark set 1 accept
> | }
> | # Removing this chain makes the first connection to succeed
> | chain PREROUTING {
> | type nat hook prerouting priority 1; policy accept;
> | $ip_proto daddr $virtual_ip udp dport 8080 dnat to umgen inc mod 2 map { 0 : $ns2_ip , 1: $ns3_ip }
> | }
> | }
>
> Foundational lecture: 'accept' verdict covers the current hook only. Like
> with iptables, if you accept in e.g. PREROUTING, INPUT may still see the
> packet. 'drop' verdict OTOH discards the packet, so no following hooks
> will see it (obviously).
>
> Your case is special because of the different types. If I interpret this
> correctly, a new connection's packet will get tproxied by divert chain
> and dnatted by PREROUTING chain (which sets up a conntrack entry). The
> second packet will hit conntrack in prerouting hook at priority -200
> (according to the big picture[1]) and your tproxy rule does not match
> anymore. The nat type chain does not see the packet as it's not a new
> connection. Maybe this explains the behaviour you're seeing.
>
> In order to avoid the inadvertent dnat of tproxied packets, terminating
> the divert chain's rule with 'drop' instead of 'accept' should do - if
> tproxy fails, it set NFT_BREAK so the packet continues and hits
> PREROUTING chain (if the connection is new).
Awesome, I just add a rule that drops the tproxied marked packets
| meta mark 1 drop
and works perfectly.
Although, since it is not really intuitive you have to do this it will
be nice to have it documented
Thanks
>
> Cheers, Phil
>
> [1] https://people.netfilter.org/pablo/nf-hooks.png
