netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Florian Westphal <fw@strlen.de>
To: Ali Abdallah <ali.abdallah@suse.com>
Cc: netfilter-devel@vger.kernel.org
Subject: Re: RSTs being marked as invalid because of wrong td_maxack value
Date: Fri, 23 Apr 2021 18:26:28 +0200	[thread overview]
Message-ID: <20210423162628.GB18119@breakpoint.cc> (raw)
In-Reply-To: <20210423155443.fmlbssgi6pq7nfp4@Fryzen495>

Ali Abdallah <ali.abdallah@suse.com> wrote:
> 1: 2049 → 703 [RST, ACK] Seq=1202969688 Ack=1132949130
> 2: [TCP Port numbers reused] 703 → 2049 [SYN] Seq=1433611541
> 3: [TCP Out-Of-Order] 703 → 2049 [PSH, ACK] Seq=1132949130 Ack=1202969688
> 4: 2049 → 703 [RST, ACK] Seq=0 Ack=1433611542

What is generating this sequence of events?  Is #3 just delayed?

> The RST in 4 is dropped, printing out the td_maxack value, it turns out
> to be:
> 
> nf_ct_tcp: invalid RST seq:0 td_maxack:1202969688 SRC=10.78.206.110
> DST=10.78.202.146 LEN=40 TOS=0x00 PREC=0x00 TTL=64 ID=43722 DF PROTO=TCP
> SPT=2049 DPT=703 SEQ=0 ACK=1433611542 WINDOW=0 RES=0x00 ACK RST URGP=0
> 
> So basically the SYN in 2 resets the IP_CT_TCP_FLAG_MAXACK_SET, while
> the out of order frame 3 resets it back, and we end up having again
> td_maxack=1202969688, that is compared against Seq=0 and the RST is dropped.

Is this after we let a SYN through while conntrack is still in
established state?

That would imply #1 was ignored too, else this should have destroyed
the entry.

> While we are still testing a proper fix, we would like to have the RST
> check introduced in [1] tunable. I can send a patch to add a proc bit
> for that, but I'm wondering whether or not to re-use the tcp_be_liberal
> option. Please let me know which option would work best for you.

Yes, be_liberal is good for this, but nevertheless I'd like to have it
behave correctly out-of-the-box.

Consider sending a new patch to add a be-liberal check for this.

Problem is that if #1 is ignored, then at #3 we can't easily know if
the syn was bogus (ack is for established connection, still alive)
or if there was re-use (ack is delayed).

Do these problematic connections support tcp timestamps?
If so, we might want to track those so we can check if the segment
is older than what we last saw.

Only problem is that this increases conn size by 16 bytes, but that
would be acceptable if that solves the problem.

  reply	other threads:[~2021-04-23 16:26 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-23 15:54 RSTs being marked as invalid because of wrong td_maxack value Ali Abdallah
2021-04-23 16:26 ` Florian Westphal [this message]
2021-04-26  7:57   ` Ali Abdallah
2021-04-26 10:11     ` Florian Westphal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210423162628.GB18119@breakpoint.cc \
    --to=fw@strlen.de \
    --cc=ali.abdallah@suse.com \
    --cc=netfilter-devel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).