google / google/gvisor

TCP conntrack does not honor window sizes correctly.

Open
#6,734 9 comments 0 reactions 0 assignees View on GitHub
revived type: bug
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.