On Thu, Mar 7, 2024 at 8:00 PM Florian Westphal <fw@xxxxxxxxx> wrote:
>
> Jason Xing <kerneljasonxing@xxxxxxxxx> wrote:
> > > This change disables most of the tcp_in_window() test, this will
> > > pretend everything is fine even though tcp_in_window says otherwise.
> >
> > Thanks for the information. It does make sense.
> >
> > What I've done is quite similar to nf_conntrack_tcp_be_liberal sysctl
> > knob which you also pointed out. It also pretends to ignore those
> > out-of-window skbs.
> >
> > >
> > > You could:
> > > - drop invalid tcp packets in input hook
> >
> > How about changing the return value only as below? Only two cases will
> > be handled:
> >
> > diff --git a/net/netfilter/nf_conntrack_proto_tcp.c
> > b/net/netfilter/nf_conntrack_proto_tcp.c
> > index ae493599a3ef..c88ce4cd041e 100644
> > --- a/net/netfilter/nf_conntrack_proto_tcp.c
> > +++ b/net/netfilter/nf_conntrack_proto_tcp.c
> > @@ -1259,7 +1259,7 @@ int nf_conntrack_tcp_packet(struct nf_conn *ct,
> > case NFCT_TCP_INVALID:
> > nf_tcp_handle_invalid(ct, dir, index, skb, state);
> > spin_unlock_bh(&ct->lock);
> > - return -NF_ACCEPT;
> > + return -NF_DROP;
>
> Lets not do this. conntrack should never drop packets and defer to ruleset
> whereever possible.
Hmm, sorry, it is against my understanding.
If we cannot return -NF_DROP, why have we already added some 'return
NF_DROP' in the nf_conntrack_handle_packet() function? And why does
this test statement exist?
nf_conntrack_in()
-> nf_conntrack_handle_packet()
-> if (ret <= 0) {
if (ret == -NF_DROP) NF_CT_STAT_INC_ATOMIC(state->net, drop);
>
> > > - set nf_conntrack_tcp_be_liberal=1
> >
> > Sure, it can workaround this case, but I would like to refuse the
> > out-of-window in netfilter or TCP layer as default instead of turning
> > on this sysctl knob. If I understand wrong, please correct me.
>
> Thats contradictory, you make a patch to always accept, then another
> patch to always drop such packets?
My only purpose is not to let the TCP layer sending strange RST to the
right flow.
Besides, resorting to turning on nf_conntrack_tcp_be_liberal sysctl
knob seems odd to me though it can workaround :S
I would like to prevent sending such an RST as default behaviour.
I wonder why we have to send RST at last due to out-of-window skbs. It
should not happen, right? As I said before, It can be set as default
without relying on some sysctl knob.
Forgive my superficial knowledge :(
>
> You can get the drop behaviour via '-m conntrack --ctstate DROP' in
> prerouting or inut hooks.
>
> You can get the 'accept + do nat processing' via
> nf_conntrack_tcp_be_liberal=1.
Sure. Just turning on the sysctl knob can be helpful because I've
tested it in production. After all, it roughly returns NFCT_TCP_ACCEPT
in nf_tcp_log_invalid() without considering those various
out-of-window cases.
Thanks,
Jason
