Re: PROBLEM: Injected conntrack lost helper

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Tue, Feb 01, 2022 at 01:04:54PM +0100, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Tue, Feb 01, 2022 at 10:08:55AM +0700, Pham Thanh Tuyen wrote:
> > > When the conntrack is created, the extension is created before the conntrack
> > > is assigned confirmed and inserted into the hash table. But the function
> > > ctnetlink_setup_nat() causes loss of helper in the mentioned situation. I
> > > mention the template because it's seamless in the
> > > __nf_ct_try_assign_helper() function. Please double check.
> > 
> > Conntrack entries that are created via ctnetlink as IPS_CONFIRMED always
> > set on.
> >
> > The helper code is only exercised from the packet path for conntrack
> > entries that are newly created.
> 
> I suspect this is the most simple fix, might make sense to also
> update the comment of IPS_HELPER to say that it means 'explicitly
> attached via ctnetlink or ruleset'.
> 
> diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
> --- a/net/netfilter/nf_conntrack_netlink.c
> +++ b/net/netfilter/nf_conntrack_netlink.c
> @@ -2313,6 +2313,9 @@ ctnetlink_create_conntrack(struct net *net,
>  
>  			/* not in hash table yet so not strictly necessary */
>  			RCU_INIT_POINTER(help->helper, helper);
> +
> +			/* explicitly attached from userspace */
> +			ct->status |= IPS_HELPER;
>  		}
>  	} else {
>  		/* try an implicit helper assignation */

This also LGTM.

I'd suggest you update the .h file to describe that ctnetlink also
sets on this bit.

Thanks