TCP conntrack does not honor window sizes correctly.
- Dominant language
- Go
- Stars
- 19.3k
- Forks
- 2k
- Avg merge
- 3d 5h
- Merged PRs (30d)
- 264
Description
### Description
tcpconntrack package uses the segment's window size specified in the TCP segment header to determine if an incoming segment is acceptable. But it doesn't take into account any window scaling that might be in use as it completely ignores TCP options in the SYN(see: https://github.com/google/gvisor/blob/81f284f9d4af4170c86ad182e2f6f2cc9b3c565a/pkg/tcpip/transport/tcpconntrack/tcp_conntrack.go#L70).
Which means the check here https://github.com/google/gvisor/blob/81f284f9d4af4170c86ad182e2f6f2cc9b3c565a/pkg/tcpip/transport/tcpconntrack/tcp_conntrack.go#L227 will incorrectly reject packets as unacceptable for anything that might lie beyond the scaled down value of the *actual window*.
It also means this check https://github.com/google/gvisor/blob/81f284f9d4af4170c86ad182e2f6f2cc9b3c565a/pkg/tcpip/transport/tcpconntrack/tcp_conntrack.go#L227 will incorrectly determine the end of the acceptable window resulting in the FIN sequence being incorrect here
https://github.com/google/gvisor/blob/81f284f9d4af4170c86ad182e2f6f2cc9b3c565a/pkg/tcpip/transport/tcpconntrack/tcp_conntrack.go#L266
Which also means that the s.closed() will incorrectly return true once the sequence number that *we incorrectly assumed to be the fin sequence number* is ACKed.
TLDR; we need to parse options in the SYN/SYN-ACK to determine if the window scaling is in effect and apply appropriate window scaling in each direction as its not required to be symmetric when calculating the actual window size.
### Steps to reproduce
_No response_
### runsc version
_No response_
### docker version (if using docker)
_No response_
### uname
_No response_
### kubectl (if using Kubernetes)
_No response_
### repo state (if built from source)
_No response_
### runsc debug logs (if available)
_No response_
Contributor guide
Assessment
This issue has not been assessed yet.