Re: [PATCH nf 2/4] netfilter: nft_flow_offload: do not make un-established tcp flow_offload session.

[Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>]

On Sat, Apr 27, 2019 at 01:53:27AM +0900, Taehee Yoo wrote:
> nft_flow_offload_eval() makes flow_offload session but it doesn't check
> tcp state.
> So, it can make un-ESTABLISHED tcp flow offload session such as SYN-RECV.
> But, this is not a normal case.
> 
> Fixes: a3c90f7a2323 ("netfilter: nf_tables: flow offload expression")
> Signed-off-by: Taehee Yoo <ap420073@xxxxxxxxx>
> ---
>  net/netfilter/nft_flow_offload.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
> index ff50bc1b144f..8538ddf9c6bf 100644
> --- a/net/netfilter/nft_flow_offload.c
> +++ b/net/netfilter/nft_flow_offload.c
> @@ -84,6 +84,9 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,
>  
>  	switch (ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.protonum) {
>  	case IPPROTO_TCP:
> +		if (ct->proto.tcp.state != TCP_CONNTRACK_ESTABLISHED)
> +			goto out;

You can restrict this via policy:

        ct status assured flow add @x

I've been considering to remove this defensive check

diff --git a/net/netfilter/nft_flow_offload.c
b/net/netfilter/nft_flow_offload.c
index 6e6b9adf7d38..bb8ea4cefc34 100644
--- a/net/netfilter/nft_flow_offload.c
+++ b/net/netfilter/nft_flow_offload.c
@@ -94,10 +94,6 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,
        if (help)
                goto out;
 
-       if (ctinfo == IP_CT_NEW ||
-           ctinfo == IP_CT_RELATED)
-               goto out;
-
        if (test_and_set_bit(IPS_OFFLOAD_BIT, &ct->status))
                goto out;

Given that we can restrict this via policy, ie.

        ct state established flow add @x

And this would fix the flowtable infrastructure UDP traffic that only
goes in one direction.

We can document in the manpage a few examples for this.