Seeking help for implementing CT HELPER in nftables

Seeking help for implementing CT HELPER in nftables

[christophe leroy]

Hello,

I'm trying to implement support for CT HELPERs in linux kernel for 
nftables and need some help/guidance.

The rule beeing 'udp dport tftp ct helper set "tftp"', I get 
nft_ct_set_init() called when I add the rule in the table output filter 
table.

I believe I have to call nf_ct_helper_ext_add() from nft_ct_set_init(), 
but for that I need the name of the helper that is to be set, ie 'tftp'.
How do I get the name of the requested helper in that function ? I 
suppose once I get it I can do the same as  xt_ct_set_helper() does.

Christophe

---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Seeking help for implementing CT HELPER in nftables

[Patrick McHardy]

On 05.03, christophe leroy wrote:
> Hello,
> 
> I'm trying to implement support for CT HELPERs in linux kernel for
> nftables and need some help/guidance.
> 
> The rule beeing 'udp dport tftp ct helper set "tftp"', I get
> nft_ct_set_init() called when I add the rule in the table output filter
> table.
> 
> I believe I have to call nf_ct_helper_ext_add() from nft_ct_set_init(),
> but for that I need the name of the helper that is to be set, ie 'tftp'.
> How do I get the name of the requested helper in that function ? I
> suppose once I get it I can do the same as  xt_ct_set_helper() does.

This depends on how we want to implement this. We could pass a static helper
name in a new CT attribute, look the helper up in the init path and assign it
to the conntrack in the eval function. This means we'd require a single rule
for every helper assignment.

Alternatively we could pass the helper name in a register and do the lookup
at runtime, that would allow to use maps for helper assignment, IOW:

ct set helper tcp dport { 21 : ftp, 5060 : sip, ... }

Since helper assignment should probably also do a module load, I'm leaning
towards the first possibility. If we still want to do map based assignements,
we could use maps of CT expressions, similar to what we do with dynset but
with statically initialized expressions.

Re: Seeking help for implementing CT HELPER in nftables

[Christophe Leroy]

Le 07/03/2016 14:20, Patrick McHardy a écrit :
> On 05.03, christophe leroy wrote:
>> Hello,
>>
>> I'm trying to implement support for CT HELPERs in linux kernel for
>> nftables and need some help/guidance.
>>
>> The rule beeing 'udp dport tftp ct helper set "tftp"', I get
>> nft_ct_set_init() called when I add the rule in the table output filter
>> table.
>>
>> I believe I have to call nf_ct_helper_ext_add() from nft_ct_set_init(),
>> but for that I need the name of the helper that is to be set, ie 'tftp'.
>> How do I get the name of the requested helper in that function ? I
>> suppose once I get it I can do the same as  xt_ct_set_helper() does.
> This depends on how we want to implement this. We could pass a static helper
> name in a new CT attribute, look the helper up in the init path and assign it
> to the conntrack in the eval function. This means we'd require a single rule
> for every helper assignment.
How do we add a new CT attribute for that ? Is there any exemple in 
other parts of the kernel for doing that ?
Is it just to add a NFTA_CT_HELPER then add it in the nft_ct_policy 
structure as an NLA_STRING type and then retrieve it with nla_strl_cpy() ?
But how does it gets populated with the helper string passed in by nft ?

Christophe


--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Seeking help for implementing CT HELPER in nftables

[Christophe Leroy]

Patrick, can you help ?

Le 09/03/2016 00:25, Christophe Leroy a écrit :
>
> Le 07/03/2016 14:20, Patrick McHardy a écrit :
>> On 05.03, christophe leroy wrote:
>>> Hello,
>>>
>>> I'm trying to implement support for CT HELPERs in linux kernel for
>>> nftables and need some help/guidance.
>>>
>>> The rule beeing 'udp dport tftp ct helper set "tftp"', I get
>>> nft_ct_set_init() called when I add the rule in the table output filter
>>> table.
>>>
>>> I believe I have to call nf_ct_helper_ext_add() from nft_ct_set_init(),
>>> but for that I need the name of the helper that is to be set, ie 
>>> 'tftp'.
>>> How do I get the name of the requested helper in that function ? I
>>> suppose once I get it I can do the same as  xt_ct_set_helper() does.
>> This depends on how we want to implement this. We could pass a static 
>> helper
>> name in a new CT attribute, look the helper up in the init path and 
>> assign it
>> to the conntrack in the eval function. This means we'd require a 
>> single rule
>> for every helper assignment.
> How do we add a new CT attribute for that ? Is there any exemple in 
> other parts of the kernel for doing that ?
> Is it just to add a NFTA_CT_HELPER then add it in the nft_ct_policy 
> structure as an NLA_STRING type and then retrieve it with 
> nla_strl_cpy() ?
> But how does it gets populated with the helper string passed in by nft ?
>
> Christophe
>
>

Christophe
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Seeking help for implementing CT HELPER in nftables

[Florian Westphal]

Christophe Leroy <christophe.leroy@c-s.fr> wrote:

[ nft_ct helper set support ]

> Patrick, can you help ?

I have a few pending patches, one of them adds an immediate
attr for ctlabel set support.

Lets see if that approach is sane enough to be reused for helper
support.

I will post it soon.

Re: Seeking help for implementing CT HELPER in nftables

[Christophe Leroy]

Hello Florian and Patrick,

Le 12/04/2016 à 15:51, Florian Westphal a écrit :
> Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>
> [ nft_ct helper set support ]
>
>> Patrick, can you help ?
>
> I have a few pending patches, one of them adds an immediate
> attr for ctlabel set support.
>
> Lets see if that approach is sane enough to be reused for helper
> support.
>
> I will post it soon.
>

I had a look but as far as I understood, the ctlabel works with bits.

For ct helper I need to retrieve the helper's name string in the 
nft_ct_set_init() function in order to call nf_ct_helper_ext_add()

Patrick suggested to add a new CT attribute, but I've not been able to 
find what has to be done for that exactly.

Is there any exemple in other parts of the kernel for doing that ?
Is it just to add a NFTA_CT_HELPER then add it in the nft_ct_policy 
structure as an NLA_STRING type and then retrieve it with nla_strl_cpy() ?
But how does it gets populated with the helper string passed in by nft ?

Thanks for your help,
Christophe

Re: Seeking help for implementing CT HELPER in nftables

[Florian Westphal]

Christophe Leroy <christophe.leroy@c-s.fr> wrote:
> Hello Florian and Patrick,
> 
> Le 12/04/2016 à 15:51, Florian Westphal a écrit :
> >Christophe Leroy <christophe.leroy@c-s.fr> wrote:
> >
> >[ nft_ct helper set support ]
> >
> >>Patrick, can you help ?
> >
> >I have a few pending patches, one of them adds an immediate
> >attr for ctlabel set support.
> >
> >Lets see if that approach is sane enough to be reused for helper
> >support.
> >
> >I will post it soon.
> >
> 
> I had a look but as far as I understood, the ctlabel works with bits.

The immediate idea was tossed and we ended up using SREG just like mark.

> For ct helper I need to retrieve the helper's name string in the
> nft_ct_set_init() function in order to call nf_ct_helper_ext_add()
> 
> Patrick suggested to add a new CT attribute, but I've not been able to find
> what has to be done for that exactly.

> Is there any exemple in other parts of the kernel for doing that ?
> Is it just to add a NFTA_CT_HELPER then add it in the nft_ct_policy

add NFTA_CT_HELPER to nft_ct_attributes, add to nft_ct_policy, yes.

> structure as an NLA_STRING type and then retrieve it with nla_strl_cpy() ?
> But how does it gets populated with the helper string passed in by nft ?

nft will need to populate this (or rather, libnftnl will do this on
behalf of nft).

Currently we do this:
nft --debug=netlink add rule filter i ct helper set foo
ip filter i
  [ immediate reg 1 0x006f6f66 0x00000000 0x00000000 0x00000000 ]
  [ ct set helper with reg 1 ]

So the string ('foo') turns into immediate and ct set uses the register.

I'd suggest to change netlink_gen_ct_stmt() (in nftables
src/netlink_linearize.c) to skip register allocation and pass the
expr string directly instead.

Perhaps one could add a function similar to

bool ct_stmt_uses_register(const struct stmt *stmt);

It would return false in case key is NFT_CT_HELPER so the linearization
step would not allocate a register and also skip the immediate
expression (and it keeps the ct details wrt. what needs the register
allocation out of the netlink code).

Instead, you would use nftnl_expr_set_str(nle, NFTNL_EXPR_CT_HELPER_NAME
to pass the string expression content to the kernel.

For reverse, you will need to make netlink_parse_ct_stmt not fail when
no register is present and create a immediate/string instead using
what is in the NFTNL_EXPR_CT_HELPER_NAME attribute.

Thanks,
Florian

Re: Seeking help for implementing CT HELPER in nftables

[Christophe Leroy]

Le 20/09/2016 à 17:38, Florian Westphal a écrit :
> Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>> Hello Florian and Patrick,
>>
>> Le 12/04/2016 à 15:51, Florian Westphal a écrit :
>>> Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>>>
>>> [ nft_ct helper set support ]
>>>
>>>> Patrick, can you help ?
>>>
>>> I have a few pending patches, one of them adds an immediate
>>> attr for ctlabel set support.
>>>
>>> Lets see if that approach is sane enough to be reused for helper
>>> support.
>>>
>>> I will post it soon.
>>>
>>
>> I had a look but as far as I understood, the ctlabel works with bits.
>
> The immediate idea was tossed and we ended up using SREG just like mark.
>
>> For ct helper I need to retrieve the helper's name string in the
>> nft_ct_set_init() function in order to call nf_ct_helper_ext_add()
>>
>> Patrick suggested to add a new CT attribute, but I've not been able to find
>> what has to be done for that exactly.
>
>> Is there any exemple in other parts of the kernel for doing that ?
>> Is it just to add a NFTA_CT_HELPER then add it in the nft_ct_policy
>
> add NFTA_CT_HELPER to nft_ct_attributes, add to nft_ct_policy, yes.
>
>> structure as an NLA_STRING type and then retrieve it with nla_strl_cpy() ?
>> But how does it gets populated with the helper string passed in by nft ?
>
> nft will need to populate this (or rather, libnftnl will do this on
> behalf of nft).
>
> Currently we do this:
> nft --debug=netlink add rule filter i ct helper set foo
> ip filter i
>   [ immediate reg 1 0x006f6f66 0x00000000 0x00000000 0x00000000 ]
>   [ ct set helper with reg 1 ]
>
> So the string ('foo') turns into immediate and ct set uses the register.
>
> I'd suggest to change netlink_gen_ct_stmt() (in nftables
> src/netlink_linearize.c) to skip register allocation and pass the
> expr string directly instead.
>
> Perhaps one could add a function similar to
>
> bool ct_stmt_uses_register(const struct stmt *stmt);
>
> It would return false in case key is NFT_CT_HELPER so the linearization
> step would not allocate a register and also skip the immediate
> expression (and it keeps the ct details wrt. what needs the register
> allocation out of the netlink code).
>
> Instead, you would use nftnl_expr_set_str(nle, NFTNL_EXPR_CT_HELPER_NAME
> to pass the string expression content to the kernel.
>
> For reverse, you will need to make netlink_parse_ct_stmt not fail when
> no register is present and create a immediate/string instead using
> what is in the NFTNL_EXPR_CT_HELPER_NAME attribute.
>

Is that really needed to do so many modifications and especially modify 
the NETLINK interface ?

Don't we have a way to retrieve the helper name from the immediate 
register in the nft_ct_set_init() function ?

I've seen that the string is available in nft_ct_set_eval() through 
&regs->data[priv->sreg], but that's too late. nft_ct_set_eval() is 
called when we get the first IP packet, not when adding the rule.

The immediate register is handled prior to calling nft_ct_set_init(), so 
there must be a way to get access to it from nft_ct_set_init(), no ?

Thanks
Christophe

Re: Seeking help for implementing CT HELPER in nftables

[Pablo Neira Ayuso]

On Fri, Sep 23, 2016 at 12:45:06PM +0200, Christophe Leroy wrote:
> Le 20/09/2016 à 17:38, Florian Westphal a écrit :
[...]
> >nft will need to populate this (or rather, libnftnl will do this on
> >behalf of nft).
> >
> >Currently we do this:
> >nft --debug=netlink add rule filter i ct helper set foo
> >ip filter i
> >  [ immediate reg 1 0x006f6f66 0x00000000 0x00000000 0x00000000 ]

Florian, Christophe, sorry for this late jump on this.

If we pass the helper name as string, then helper autoload will not
work as we don't have a way to solve this from the packet path.

To solve this, I'm considering a different approach. Basically,
explicit preload the helpers and pass a helper handle through
register instead.

In the ruleset file, this would look like this:

        table ip x {
                helper ftp protocol tcp                 #1

                chain y {
                        ...
                        tcp dport 21 ct helper set ftp  #2
                }
        }

Line #1 makes sure the ftp helper is loaded, we also increment reference
counter. This results in a handle that is dynamically allocated by
nf_tables, that can be retrieve in the same fashion of if_index (ie.
we can look up for the handle from the helper name).

Then from #2, we use the helper handle to refer to the helper.

Good things I see on this:

1) We can still use maps, although we would need a function to lookup
   for the nf_conntrack_helper struct form this handle. We can have a
   small hashtable for this (actually, we have similar approach currently
   to look up for helpers from ports in the deprecated automagic helper
   as signment).
2) We use a u32 instead of string to identify the helper.
3) We solve the module autoload issue.
4) We make sure helper module cannot be removed while there is a
   reference from the ruleset.
5) We can validate that the helper runs from the right layer 4
   protocol, eg. ensure the ftp helper is called for tcp since we can
   do this from nft evaluation phase.

This would need commands like:

        nft add helper ip x name ftp protocol tcp

and to delete this:

        nft delete helper ip x name ftp protocol tcp

Main drawback is that this needs a explicit definition of the helper,
but I think this extra line is ok if it helps resolve other problems.

Re: Seeking help for implementing CT HELPER in nftables

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Fri, Sep 23, 2016 at 12:45:06PM +0200, Christophe Leroy wrote:
> > Le 20/09/2016 à 17:38, Florian Westphal a écrit :
> [...]
> > >nft will need to populate this (or rather, libnftnl will do this on
> > >behalf of nft).
> > >
> > >Currently we do this:
> > >nft --debug=netlink add rule filter i ct helper set foo
> > >ip filter i
> > >  [ immediate reg 1 0x006f6f66 0x00000000 0x00000000 0x00000000 ]
> 
> Florian, Christophe, sorry for this late jump on this.
> 
> If we pass the helper name as string, then helper autoload will not
> work as we don't have a way to solve this from the packet path.

The point of passing it as a string was to have it available at .init()
time so we can look up the helper and do a modprobe if needed.

I agree wrt. to maps though, that goes out the door since no sreg is
used anymore.

[..]

> Line #1 makes sure the ftp helper is loaded, we also increment reference
> counter. This results in a handle that is dynamically allocated by
> nf_tables, that can be retrieve in the same fashion of if_index (ie.
> we can look up for the handle from the helper name).
> 
> Then from #2, we use the helper handle to refer to the helper.

Hmm, Christophe asked for a simpler solution ;)

Re: Seeking help for implementing CT HELPER in nftables

[Christophe Leroy]

Le 23/09/2016 à 16:24, Pablo Neira Ayuso a écrit :
> On Fri, Sep 23, 2016 at 12:45:06PM +0200, Christophe Leroy wrote:
>> Le 20/09/2016 à 17:38, Florian Westphal a écrit :
> [...]
>>> nft will need to populate this (or rather, libnftnl will do this on
>>> behalf of nft).
>>>
>>> Currently we do this:
>>> nft --debug=netlink add rule filter i ct helper set foo
>>> ip filter i
>>>  [ immediate reg 1 0x006f6f66 0x00000000 0x00000000 0x00000000 ]
>
> Florian, Christophe, sorry for this late jump on this.
>
> If we pass the helper name as string, then helper autoload will not
> work as we don't have a way to solve this from the packet path.

That's maybe a stupid idea, but my idea was to do the same as 
xt_ct_set_helper() from nft_ct_set_init(), hence the need to be able to 
catch the name in the _init() function.

By doing this, the helper would be autoloaded if needed, wouldn't it ?

Christophe

>
> To solve this, I'm considering a different approach. Basically,
> explicit preload the helpers and pass a helper handle through
> register instead.
>
> In the ruleset file, this would look like this:
>
>         table ip x {
>                 helper ftp protocol tcp                 #1
>
>                 chain y {
>                         ...
>                         tcp dport 21 ct helper set ftp  #2
>                 }
>         }
>
> Line #1 makes sure the ftp helper is loaded, we also increment reference
> counter. This results in a handle that is dynamically allocated by
> nf_tables, that can be retrieve in the same fashion of if_index (ie.
> we can look up for the handle from the helper name).
>
> Then from #2, we use the helper handle to refer to the helper.
>
> Good things I see on this:
>
> 1) We can still use maps, although we would need a function to lookup
>    for the nf_conntrack_helper struct form this handle. We can have a
>    small hashtable for this (actually, we have similar approach currently
>    to look up for helpers from ports in the deprecated automagic helper
>    as signment).
> 2) We use a u32 instead of string to identify the helper.
> 3) We solve the module autoload issue.
> 4) We make sure helper module cannot be removed while there is a
>    reference from the ruleset.
> 5) We can validate that the helper runs from the right layer 4
>    protocol, eg. ensure the ftp helper is called for tcp since we can
>    do this from nft evaluation phase.
>
> This would need commands like:
>
>         nft add helper ip x name ftp protocol tcp
>
> and to delete this:
>
>         nft delete helper ip x name ftp protocol tcp
>
> Main drawback is that this needs a explicit definition of the helper,
> but I think this extra line is ok if it helps resolve other problems.
>

Re: Seeking help for implementing CT HELPER in nftables

[Pablo Neira Ayuso]

On Fri, Sep 23, 2016 at 04:48:32PM +0200, Christophe Leroy wrote:
> Le 23/09/2016 à 16:24, Pablo Neira Ayuso a écrit :
> >On Fri, Sep 23, 2016 at 12:45:06PM +0200, Christophe Leroy wrote:
> >>Le 20/09/2016 à 17:38, Florian Westphal a écrit :
> >[...]
> >>>nft will need to populate this (or rather, libnftnl will do this on
> >>>behalf of nft).
> >>>
> >>>Currently we do this:
> >>>nft --debug=netlink add rule filter i ct helper set foo
> >>>ip filter i
> >>> [ immediate reg 1 0x006f6f66 0x00000000 0x00000000 0x00000000 ]
> >
> >Florian, Christophe, sorry for this late jump on this.
> >
> >If we pass the helper name as string, then helper autoload will not
> >work as we don't have a way to solve this from the packet path.
> 
> That's maybe a stupid idea, but my idea was to do the same as
> xt_ct_set_helper() from nft_ct_set_init(), hence the need to be able to
> catch the name in the _init() function.
>
> By doing this, the helper would be autoloaded if needed, wouldn't it ?

Yes. Something similar to xt_ct_set_helper() would autoload the
module.

Hm, but this needs more attributes, not only the helper name.
nf_conntrack_helper_try_module_get() needs l3 and l4 protocol numbers,
and this information won't be available from there unless we add
explicit netlink attributes to specify then too. This information is
important since we have helpers that run over udp and tcp.

Re: Seeking help for implementing CT HELPER in nftables

[Christophe Leroy]

Le 23/09/2016 à 17:19, Pablo Neira Ayuso a écrit :
> On Fri, Sep 23, 2016 at 04:48:32PM +0200, Christophe Leroy wrote:
>> Le 23/09/2016 à 16:24, Pablo Neira Ayuso a écrit :
>>> On Fri, Sep 23, 2016 at 12:45:06PM +0200, Christophe Leroy wrote:
>>>> Le 20/09/2016 à 17:38, Florian Westphal a écrit :
>>> [...]
>>>>> nft will need to populate this (or rather, libnftnl will do this on
>>>>> behalf of nft).
>>>>>
>>>>> Currently we do this:
>>>>> nft --debug=netlink add rule filter i ct helper set foo
>>>>> ip filter i
>>>>> [ immediate reg 1 0x006f6f66 0x00000000 0x00000000 0x00000000 ]
>>>
>>> Florian, Christophe, sorry for this late jump on this.
>>>
>>> If we pass the helper name as string, then helper autoload will not
>>> work as we don't have a way to solve this from the packet path.
>>
>> That's maybe a stupid idea, but my idea was to do the same as
>> xt_ct_set_helper() from nft_ct_set_init(), hence the need to be able to
>> catch the name in the _init() function.
>>
>> By doing this, the helper would be autoloaded if needed, wouldn't it ?
>
> Yes. Something similar to xt_ct_set_helper() would autoload the
> module.
>
> Hm, but this needs more attributes, not only the helper name.
> nf_conntrack_helper_try_module_get() needs l3 and l4 protocol numbers,
> and this information won't be available from there unless we add
> explicit netlink attributes to specify then too. This information is
> important since we have helpers that run over udp and tcp.
>


Right but the rule we add to the output filter is

udp dport tftp ct helper set "tftp"

So the l3/l4 information is there. Can it be retrieved in 
nft_ct_set_init() ?

Christophe

Re: Seeking help for implementing CT HELPER in nftables

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Fri, Sep 23, 2016 at 12:45:06PM +0200, Christophe Leroy wrote:
> > Le 20/09/2016 à 17:38, Florian Westphal a écrit :
> [...]
> > >nft will need to populate this (or rather, libnftnl will do this on
> > >behalf of nft).
> > >
> > >Currently we do this:
> > >nft --debug=netlink add rule filter i ct helper set foo
> > >ip filter i
> > >  [ immediate reg 1 0x006f6f66 0x00000000 0x00000000 0x00000000 ]
> 
> Florian, Christophe, sorry for this late jump on this.
> 
> If we pass the helper name as string, then helper autoload will not
> work as we don't have a way to solve this from the packet path.
> 
> To solve this, I'm considering a different approach. Basically,
> explicit preload the helpers and pass a helper handle through
> register instead.
> 
> In the ruleset file, this would look like this:
> 
>         table ip x {
>                 helper ftp protocol tcp                 #1

This would also allow to support helper specific configuration from the
nft frontend rather than via modprobe args (e.g. ftp loose mode).