Page MenuHomeFreeBSD

tcp: improve SEG.ACK validation
ClosedPublic

Authored by tuexen on Jul 5 2024, 11:07 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Dec 24, 2:21 AM
Unknown Object (File)
Sun, Dec 22, 4:25 PM
Unknown Object (File)
Sat, Dec 14, 5:22 PM
Unknown Object (File)
Tue, Dec 10, 12:00 AM
Unknown Object (File)
Sat, Dec 7, 3:17 PM
Unknown Object (File)
Wed, Dec 4, 3:42 AM
Unknown Object (File)
Wed, Dec 4, 3:42 AM
Unknown Object (File)
Wed, Dec 4, 3:42 AM

Details

Summary

Implement the improved SEG.ACK validation described in RFC 5961. In addition to that, also detect ghost ACKs, which are ACKs for data that has never been sent.

The additional checks are enabled by default, but can be disabled by setting the sysctl-variable net.inet.tcp.insecure-ack to a non-zero value.

This issue was reported in PR 250357.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

Improve output of netstat -sptcp.

This revision is now accepted and ready to land.Jul 11 2024, 3:38 PM

After reading the paper and IETF slides, it looks good to me on top of the general view. Just some small concerns below.

sys/netinet/tcp_input.c
2244

At the server side (listening side), I am thinking if we move the secure ACK check here and above the TOF_TS check, it would save cycles on checking these timestamps.

2447–2448

code style: As there are two lines, recommend using a pair of {} for readability.

sys/netinet/tcp_stacks/bbr.c
7715–7716

code style:
Same here. As there are two lines, recommend using a pair of {} for readability.

sys/netinet/tcp_stacks/rack.c
12476–12477

code style:
Same here. As there are two lines, recommend using a pair of {} for readability.

sys/netinet/tcp_input.c
2244

This patch is only about improving the input validation, not about changing the sequence of checks.
Please note that RFC 7323 requires to perform the timestamp checks before the SEG.ACK number checks.

Always use the stricter check and address style issues reported by cc@.

This revision now requires review to proceed.Jul 19 2024, 3:00 PM
tuexen added inline comments.
sys/netinet/tcp_input.c
2447–2448

Fixed.

sys/netinet/tcp_stacks/bbr.c
7715–7716

Fixed.

sys/netinet/tcp_stacks/rack.c
12476–12477

Fixed.

This revision is now accepted and ready to land.Jul 20 2024, 4:49 PM

Thanks for the update. Looks better and just a small concern about TCP_MAXWIN for 65535.

sys/netinet/tcp_input.c
2244

I see, in the section 5.3. Basic PAWS Algorithm from RFC7323 claims Also, PAWS processing MUST take precedence over the regular TCP acceptability check (Section 3.3 in [RFC0793]), which is performed after verification of the received Timestamps option...

2446

Can use TCP_MAXWIN instead of 65535.

tuexen added inline comments.
sys/netinet/tcp_input.c
2446

Fixed in 37b3e6a660f551ceda412e2115748f3b97b2e615, since this review has already been committed.