Re: [PATCH nf] netfilter: nft_tproxy: make it terminal

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

 



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).

Cheers, Phil

[1] https://people.netfilter.org/pablo/nf-hooks.png




[Index of Archives]     [Netfitler Users]     [Berkeley Packet Filter]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux