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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Apr 27, 2019 at 02:48:15AM +0900, Taehee Yoo wrote:
> On Sat, 27 Apr 2019 at 02:03, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> >
> 
> Hi Pablo!
> Thank you for the review!
> 
> > 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 think If we allow to make non-ESTABLISHED flow offload,
> teardown code should be changed because teardown changes TCP status to
> ESTABLISHED. How do you think about it?

Hm, right, the teardown fixup routine cannot then bogusly assumes the
TCP established state if we create a TCP entry from the first packet,
ie. the SYN_SENT state...

[...]
> > 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.
> 
> I agree that flowtable infrastructure provides UDP one direction flow
> offloading.

OK, probably I can collapse your patch to mine. Let me think over the
weekend.



[Index of Archives]     [Netfitler Users]     [Berkeley Packet Filter]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux