tun: disqualify tcp4 packets w/IP options from coalescing

IP options were not being compared prior to coalescing. They are not
commonly used. Disqualification due to nonzero options is in line with
the kernel.

Reviewed-by: Denton Gentry <dgentry@tailscale.com>
Signed-off-by: Jordan Whited <jordan@tailscale.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>

diff --git a/tun/tcp_offload_linux.go b/tun/tcp_offload_linux.go
index e807f0077bd1dd28efbcf134ceb77db13d91ef22..4912efd3f75acf08af8ee6953ea8434a83d9c705 100644 (file)
--- a/tun/tcp_offload_linux.go
+++ b/tun/tcp_offload_linux.go
@@ -397,9 +397,6 @@ func tcpGRO(bufs [][]byte, offset int, pktI int, table *tcpGROTable, isV6 bool)
                if totalLen != len(pkt) {
                        return false
                }
-               if iphLen < 20 || iphLen > 60 {
-                       return false
-               }
        }
        if len(pkt) < iphLen {
                return false
@@ -474,13 +471,16 @@ func tcpGRO(bufs [][]byte, offset int, pktI int, table *tcpGROTable, isV6 bool)
        return false
 }
 
-func isTCP4(b []byte) bool {
+func isTCP4NoIPOptions(b []byte) bool {
        if len(b) < 40 {
                return false
        }
        if b[0]>>4 != 4 {
                return false
        }
+       if b[0]&0x0F != 5 {
+               return false
+       }
        if b[9] != unix.IPPROTO_TCP {
                return false
        }
@@ -511,7 +511,7 @@ func handleGRO(bufs [][]byte, offset int, tcp4Table, tcp6Table *tcpGROTable, toW
                }
                var coalesced bool
                switch {
-               case isTCP4(bufs[i][offset:]):
+               case isTCP4NoIPOptions(bufs[i][offset:]): // ipv4 packets w/IP options do not coalesce
                        coalesced = tcpGRO(bufs, offset, i, tcp4Table, false)
                case isTCP6NoEH(bufs[i][offset:]): // ipv6 packets w/extension headers do not coalesce
                        coalesced = tcpGRO(bufs, offset, i, tcp6Table, true)

diff --git a/tun/tcp_offload_linux_test.go b/tun/tcp_offload_linux_test.go
index 11f9e53b51d7eb11fd62f683bfecf5e6925eb59b..046e177e04e4aa0c6c07fe93f20d7dc3bee17fc9 100644 (file)
--- a/tun/tcp_offload_linux_test.go
+++ b/tun/tcp_offload_linux_test.go
@@ -271,3 +271,53 @@ func Test_handleGRO(t *testing.T) {
                })
        }
 }
+
+func Test_isTCP4NoIPOptions(t *testing.T) {
+       valid := tcp4Packet(ip4PortA, ip4PortB, header.TCPFlagAck, 100, 1)[virtioNetHdrLen:]
+       invalidLen := valid[:39]
+       invalidHeaderLen := make([]byte, len(valid))
+       copy(invalidHeaderLen, valid)
+       invalidHeaderLen[0] = 0x46
+       invalidProtocol := make([]byte, len(valid))
+       copy(invalidProtocol, valid)
+       invalidProtocol[9] = unix.IPPROTO_TCP + 1
+
+       tests := []struct {
+               name string
+               b    []byte
+               want bool
+       }{
+               {
+                       "valid",
+                       valid,
+                       true,
+               },
+               {
+                       "invalid length",
+                       invalidLen,
+                       false,
+               },
+               {
+                       "invalid version",
+                       []byte{0x00},
+                       false,
+               },
+               {
+                       "invalid header len",
+                       invalidHeaderLen,
+                       false,
+               },
+               {
+                       "invalid protocol",
+                       invalidProtocol,
+                       false,
+               },
+       }
+       for _, tt := range tests {
+               t.Run(tt.name, func(t *testing.T) {
+                       if got := isTCP4NoIPOptions(tt.b); got != tt.want {
+                               t.Errorf("isTCP4NoIPOptions() = %v, want %v", got, tt.want)
+                       }
+               })
+       }
+}