net/packet, wgengine/netstack: remove workaround for old gvisor ECN bug

Fixes #2642

Change-Id: Ic02251d24a4109679645d1c8336e0f961d0cce13
Signed-off-by: Brad Fitzpatrick <[email protected]>

net/packet/packet.go

@@ -434,40 +434,6 @@ func (q *Parsed) IsEchoResponse() bool {
 	}
 }
 
-// RemoveECNBits modifies p and its underlying memory buffer to remove
-// ECN bits, if any. It reports whether it did so.
-//
-// It currently only does the TCP flags.
-func (p *Parsed) RemoveECNBits() bool {
-	if p.IPVersion == 0 {
-		return false
-	}
-	if p.IPProto != ipproto.TCP {
-		// TODO(bradfitz): handle non-TCP too? for now only trying to
-		// fix the Issue 2642 problem.
-		return false
-	}
-	if p.TCPFlags&TCPECNBits == 0 {
-		// Nothing to do.
-		return false
-	}
-
-	// Clear flags.
-
-	// First in the parsed output.
-	p.TCPFlags = p.TCPFlags & ^TCPECNBits
-
-	// Then in the underlying memory.
-	tcp := p.Transport()
-	old := binary.BigEndian.Uint16(tcp[12:14])
-	tcp[13] = byte(p.TCPFlags)
-	new := binary.BigEndian.Uint16(tcp[12:14])
-	oldSum := binary.BigEndian.Uint16(tcp[16:18])
-	newSum := ^checksumUpdate2ByteAlignedUint16(^oldSum, old, new)
-	binary.BigEndian.PutUint16(tcp[16:18], newSum)
-	return true
-}
-
 func Hexdump(b []byte) string {
 	out := new(strings.Builder)
 	for i := 0; i < len(b); i += 16 {
@@ -499,26 +465,3 @@ func Hexdump(b []byte) string {
 	}
 	return out.String()
 }
-
-// From gVisor's unexported API:
-
-// checksumUpdate2ByteAlignedUint16 updates a uint16 value in a calculated
-// checksum.
-//
-// The value MUST begin at a 2-byte boundary in the original buffer.
-func checksumUpdate2ByteAlignedUint16(xsum, old, new uint16) uint16 {
-	// As per RFC 1071 page 4,
-	//(4)  Incremental Update
-	//
-	//        ...
-	//
-	//        To update the checksum, simply add the differences of the
-	//        sixteen bit integers that have been changed.  To see why this
-	//        works, observe that every 16-bit integer has an additive inverse
-	//        and that addition is associative.  From this it follows that
-	//        given the original value m, the new value m', and the old
-	//        checksum C, the new checksum C' is:
-	//
-	//                C' = C + (-m) + m' = C + (m' - m)
-	return checksumCombine(xsum, checksumCombine(new, ^old))
-}

net/packet/packet_test.go

@@ -6,9 +6,7 @@ package packet
 
 import (
 	"bytes"
-	"encoding/hex"
 	"reflect"
-	"regexp"
 	"testing"
 
 	"inet.af/netaddr"
@@ -563,57 +561,3 @@ func BenchmarkString(b *testing.B) {
 		})
 	}
 }
-
-func TestRemoveECNBits(t *testing.T) {
-	// withECNHex is a TCP SYN packet with ECN bits set in the TCP
-	// header as captured by Wireshark on macOS against the
-	// Tailscale interface. In this packet (because it's a SYN
-	// control packet), the ECN bits are not set in the IP header.
-	const withECNHex = `45 00 00 40 00 00 40 00
- 40 06 0c 66 64 7b 65 28 64 7f 00 30 f1 ab 00 16
- 5a 7a 63 e8 00 00 00 00 b0 c2 ff ff 97 76 00 00
- 02 04 04 d8 01 03 03 06 01 01 08 0a 03 e1 bd 49
- 00 00 00 00 04 02 00 00`
-
-	// Generated by hand-editing a pcap file in hexl-mode to set
-	// the TCP flags to just SYN (0x02), then loading that pcap
-	// file in wireshark to get the expected checksum value, then
-	// putting that checksum value (0x9836) in the file.
-	const wantStrippedHex = `45 00 00 40 00 00 40 00
- 40 06 0c 66 64 7b 65 28 64 7f 00 30 f1 ab 00 16
- 5a 7a 63 e8 00 00 00 00 b0 02 ff ff 98 36 00 00
- 02 04 04 d8 01 03 03 06 01 01 08 0a 03 e1 bd 49
- 00 00 00 00 04 02 00 00`
-
-	var p Parsed
-	pktBuf := bytesOfHex(withECNHex)
-	p.Decode(pktBuf)
-	if want := TCPCWR | TCPECNEcho | TCPSyn; p.TCPFlags != want {
-		t.Fatalf("pre flags = %v; want %v", p.TCPFlags, want)
-	}
-
-	if !p.RemoveECNBits() {
-		t.Fatal("didn't remove bits")
-	}
-	if want := TCPSyn; p.TCPFlags != want {
-		t.Fatalf("post flags = %v; want %v", p.TCPFlags, want)
-	}
-	wantPkt := bytesOfHex(wantStrippedHex)
-	if !bytes.Equal(pktBuf, wantPkt) {
-		t.Fatalf("wrong result.\n got: % 2x\nwant: % 2x\n", pktBuf, wantPkt)
-	}
-
-	if p.RemoveECNBits() {
-		t.Fatal("unexpected true return value on second call")
-	}
-}
-
-var nonHex = regexp.MustCompile(`[^0-9a-fA-F]+`)
-
-func bytesOfHex(s string) []byte {
-	b, err := hex.DecodeString(nonHex.ReplaceAllString(s, ""))
-	if err != nil {
-		panic(err)
-	}
-	return b
-}

wgengine/netstack/netstack.go

@@ -563,7 +563,6 @@ func (ns *Impl) injectInbound(p *packet.Parsed, t *tstun.Wrapper) filter.Respons
 	case 6:
 		pn = header.IPv6ProtocolNumber
 	}
-	p.RemoveECNBits() // Issue 2642
 	if debugPackets {
 		ns.logf("[v2] packet in (from %v): % x", p.Src, p.Buffer())
 	}