netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Florian Westphal <fw@strlen.de>
To: pablo@netfilter.org
Cc: Florian Westphal <fw@strlen.de>,
	netfilter-devel@vger.kernel.org, christophe.leroy@c-s.fr
Subject: Re: nftables conntrack set ops for zone, helper assignment, etc.
Date: Thu, 12 Jan 2017 15:53:10 +0100	[thread overview]
Message-ID: <20170112145310.GB24985@breakpoint.cc> (raw)
In-Reply-To: <20170112142818.GA3211@salvia>

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > No, I meant more intrusive version:
> > 
> > ct = nf_ct_get(...);
> > if (!ct || nf_ct_is_template(ct))))
> > 	nf_conntrack_in(net, info->pf, hook, skb);
> 
> OK, then we have to defrag first. Let me give another twist to this
> discussion, still brainstorming.

No, nf_conntrack forces a dependency on the defrag module, so unless
someone inserts such rule before the defrag hook we're fine (raw
table prio is fine for instance).

> Did you consider to move this logic into a explicit 'track' statement.

Yes, but that doesn't help a lot imo since that forces a user
to structure ruleset so that they do

ct zone set meta mark track with helper "ftp-standalone"

which looks fine but what does the 'track' keyword provide?
Would a user care?  Or is that just more about exposing whats going on
behind the scenes?

> Instead of this implicit lookup hidden in the helper/timeout
> assignment, syntax would be something like:
> 
>         tcp dport 21 track with helper "ftp-standalone" timeout "tcp-short-timeout"
> 
> Note, that:
> 
>         track with helper "ftp-standalone" timeout "tcp-short-timeout"
> 
> performs this lookup&change as you need, but in one go, only for
> unconfirmed conntrack. And this would be achieved with one single
> kernel expression, in nft --debug=netlink representation:
> 
>  [ track helper "ftp-standalone" timeout "tcp-short-timeout" ]

I see, so we add a new expression instead of using ct set syntax.

> For zone ID, we can use the same thing:
> 
>         track with zone 1       [ This can be combined with helper/timeouts too. ]

Hmm....  So we really have comeptely new user syntax for all of
this.

The reason I scrapped this idea early on is that this breaks horribly
with icmp error messages and reply traffic (at very least, its
highly counter-intuitive/inconvenient).  Consider this fine looking
but non-working line:

tcp dport 21 track with helper "ftp-standalone" zone 1

instead, users would have to do this:

track with zone 1 # so icmp, reply traffic, ftp data traffic is in zone 1 too
tcp dport 21 track with helper "ftp-standalone"

and that won't work either since we would have to support 're-tracking'
already tracked conntrack...

I planned to force explicit 'track' for the bridge conntrack support,
but that works only because the nf_conntrack_in PREROUTING equivalent
added in bridge_conntrack would not ever create new conntracks (only
RELATED).

We can do this because there is no bridge conntrack so far so we don't
have backwards compat problems.

But then I thought 'why add special "track" if I can just let
ct foo set bla do it for me'?

> In this case, we pass the zone ID via nf_conntrack_in() [ or a new
> function that is called from nf_conntrack_in() that takes the zone ID
> and that doesn't depend on templates anymore, we can strip off
> nf_conntrack_in() from the template logic ].

I found no way to avoid templates for the zone because of the RELATED
packets.

> > > Back to helpers, users are familiar with the current way to attach
> > > helpers, ie. from the raw chain.
> > > 
> > > Am I missing anything? I'm starting to think we can't escape the
> > > conntrack template.
> > 
> > For Helpers?  Why not?  As long as ct isn't in the main table it should
> > be fine afaics?  (Unless you mean "can't escape conntrack template to
> > read to helper info that we need to assign to ct from".
> >
> > For zones, yes, I don't see a way to avoid a template for them.
> > But thats the only ct key where I think that a template has to be used.
> 
> Yes, following the approach you propose zone would be the only one
> that requires the template. So this needs to happen before the
> conntrack hook.

Yes, thats right.

      reply	other threads:[~2017-01-12 14:53 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-11  9:17 nftables conntrack set ops for zone, helper assignment, etc Florian Westphal
2017-01-11 20:07 ` Pablo Neira Ayuso
2017-01-11 23:01   ` Florian Westphal
2017-01-12 13:00     ` Pablo Neira Ayuso
2017-01-12 13:29       ` Florian Westphal
2017-01-12 14:28         ` Pablo Neira Ayuso
2017-01-12 14:53           ` Florian Westphal [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170112145310.GB24985@breakpoint.cc \
    --to=fw@strlen.de \
    --cc=christophe.leroy@c-s.fr \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pablo@netfilter.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).