Hi, On Fri, Sep 13, 2024 at 01:24:25PM +0200, Pablo Neira Ayuso wrote: > On Fri, Sep 13, 2024 at 01:02:02PM +0200, Antonio Ojea wrote: > > On Fri, 13 Sept 2024 at 12:47, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote: > > > > > > On Fri, Sep 13, 2024 at 12:41:01PM +0200, Florian Westphal wrote: > > > > Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote: > > > > > On Fri, Sep 13, 2024 at 12:23:47PM +0200, Florian Westphal wrote: > > > > > > Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote: > > > > > > > tproxy action must be terminal since the intent of the user to steal the > > > > > > > traffic and redirect to the port. > > > > > > > Align this behaviour to iptables to make it easier to migrate by issuing > > > > > > > NF_ACCEPT for packets that are redirect to userspace process socket. > > > > > > > Otherwise, NF_DROP packet if socket transparent flag is not set on. > > > > > > > > > > > > The nonterminal behaviour is intentional. This change will likely > > > > > > break existing setups. > > > > > > > > > > > > nft add rule filter divert tcp dport 80 tproxy to :50080 meta mark set 1 accept > > > > > > > > > > > > This is a documented example. > > > > > > > > > > Ouch. Example could have been: > > > > > > > > > > nft add rule filter divert tcp dport 80 socket transparent meta set 1 tproxy to :50080 > > > > > > > > Yes, but its not the same. > > > > > > > > With the statements switched, all tcp dport 80 have the mark set. > > > > With original example, the mark is set only if tproxy found a > > > > transparent sk. > > > > > > Indeed, thanks for correcting me. > > > > > > I'm remembering now why this was done to provide to address the ugly > > > mark hack that xt_TPROXY provides. > > > > > > While this is making harder to migrate, making it non-terminal is > > > allowing to make more handling such as ct/meta marking after it. > > > > > > I think we just have to document this in man nft(8). > > > > I think that at this point in time the current state can not be broken > > based on this discussion, I just left the comment in the bugzilla > > about the possibility but it is clear now that people that have > > already started using this feature with nftables must not experience a > > disruption. > > On the other side, users that need to migrate will have to adapt more > > things so I don't think it should be a big deal. > > What I really think is that users should have a way to terminate > > processing to avoid other rules to interfere with the tproxy > > functionality > > It is possible to add an explicit 'accept' verdict as the example > above displays: > > tcp dport 80 tproxy to :50080 meta mark set 1 accept > ^^^^^^ I wonder if this is sufficient: The packet will still appear in following chains, etc. So shouldn't one use 'drop' verdict instead or does that prevent the proxying somehow? Hmm. Looking at nft_nat.c, 'accept' seems consistent with what nat statements do implicitly. > 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. Also I should update the respective wiki article[1] once more with added translation testsuite links - at least the one for TPROXY is missing. Cheers, Phil
