RSTs being marked as invalid because of wrong td_maxack value

RSTs being marked as invalid because of wrong td_maxack value

[Ali Abdallah]

[-- Attachment #1: Type: text/plain, Size: 1386 bytes --]

Greetings,

We are seeing the following situation, on an established connection:

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

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.

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.

Thanks in advance.

[1] https://patchwork.ozlabs.org/project/netdev/patch/20090527143523.4649.91602.sendpatchset@x2.localnet/

-- 
Ali Abdallah | SUSE Linux L3 Engineer
GPG fingerprint: 51A0 F4A0 C8CF C98F 842E  A9A8 B945 56F8 1C85 D0D5


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: RSTs being marked as invalid because of wrong td_maxack value

[Florian Westphal]

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.

Re: RSTs being marked as invalid because of wrong td_maxack value

[Ali Abdallah]

On 23.04.2021 18:26, Florian Westphal wrote:
> What is generating this sequence of events?  Is #3 just delayed?

Yes.

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

Yes.

> That would imply #1 was ignored too, else this should have destroyed
> the entry.
> 
> 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.

Ok perfect, will send a patch later then.

> 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.

Yes, they do support timestamps, but I'm not sure if that will solve
the problem, as the problematic out of order frame #5 has more recent
timestamps.

1: 703 → 2049 [ACK] Seq=1132947682 Ack=1202969688 Win=2661 Len = 0
TSval=3506781692 TSecr=1717385629

2: 2049 → 703 [RST, ACK] Seq=1202969688 Ack=1132949130 Win=69120 Len=0
TSval=1717385630 TSecr=3506781692

3: 2049 → 703 [ACK] Seq=1202969688 Ack=1132947682 Win=70400 Len=0
TSval=1717385630 TSecr=3506781692

4: 703 → 2049 [SYN] Seq=1433611541 Win=29200 Len=0 MSS=1460 SACK_PERM=1
TSval=3506781693 TSecr=0

5: [PSH, ACK] Seq=1132949130 Ack=1202969688 Win=1362432 Len=604
TSval=3506781693 TSecr=1717385629

6: 2049 → 703 [RST, ACK] Seq=0 Ack=1433611542 Win=0 Len=0


Regards,

-- 
Ali Abdallah | SUSE Linux L3 Engineer
GPG fingerprint: 51A0 F4A0 C8CF C98F 842E  A9A8 B945 56F8 1C85 D0D5

Re: RSTs being marked as invalid because of wrong td_maxack value

[Florian Westphal]

Ali Abdallah <ali.abdallah@suse.com> wrote:
> Yes, they do support timestamps, but I'm not sure if that will solve
> the problem, as the problematic out of order frame #5 has more recent
> timestamps.
> 
> 1: 703 → 2049 [ACK] Seq=1132947682 Ack=1202969688 Win=2661 Len = 0
> TSval=3506781692 TSecr=1717385629
> 
> 2: 2049 → 703 [RST, ACK] Seq=1202969688 Ack=1132949130 Win=69120 Len=0
> TSval=1717385630 TSecr=3506781692

Ok, right, I forgot we may have different clients behind NAT.