Patrick/tamper

.golangci.yml

@@ -19,6 +19,10 @@ linters:
     - ireturn
     - nolintlint
     - varnamelen
+    - depguard
+    - dupword
+    - testpackage
+    - godox
 
 linters-settings:
   misspell:
@@ -28,10 +32,34 @@ linters-settings:
     block-size: 2
   exhaustive:
     default-signifies-exhaustive: true
+  gosec:
+    excludes:
+      - G104 # Duplicated errcheck checks
+      - G404 # Use of math/rand for RNG
+  wrapcheck:
+    ignoreSigs:
+      - .Errorf(
+      - errors.New(
+      - errors.Unwrap(
+      - .Apply(
+  revive:
+    rules:
+      - name: unused-parameter
+        severity: warning
+        disabled: true
+        arguments:
+          - allowRegex: "^_"
 
 issues:
   exclude-rules:
     # False positive: https://github.com/kunwardeep/paralleltest/issues/8.
     - linters:
       - paralleltest
       text: "does not use range value in test Run"
+    - linters:
+      - errcheck
+      text: "Error return value of .(tcp|ip|udp).SerializeTo. is not checked"
+  exclude:
+    # these should be self-documenting
+    - "exported const ((TCP|IPv4)(Field|Flag|Option).*|IPFieldVersion) should (have comment|be of the form)"
+    - "do not define dynamic errors, use wrapped static errors instead:"

README.md

@@ -103,6 +103,13 @@ that "branching actions are not supported on inbound trees". Practically, this m
 actions can only be applied to outbound packets, while the sleep, drop, and tamper actions can apply to packets of
 either direction. (Note that this package does not currently enforce this; this is also a bug.)
 
+## Disclaimer
+
+Currently only IPv4 and TCP are supported. There are plans to add support for UDP in the future (although pull requests
+are welcome! Look at `TCPTamperAction` and `IPv4TamperAction` in `actions/tamper_action.go` as examples.
+`UDPTamperAction` must implement the `actions.Action` interface). There are no plans at the moment to add support 
+for IPv6.
+
 ## Credits
 
 See https://censorship.ai for more information about Geneva itself.

actions/action_test.go

@@ -1,14 +1,15 @@
 package actions_test
 
 import (
-	"encoding/binary"
 	"fmt"
 	"testing"
 
-	"github.com/getlantern/geneva/actions"
-	"github.com/getlantern/geneva/internal/scanner"
 	"github.com/google/gopacket"
 	"github.com/google/gopacket/layers"
+
+	"github.com/getlantern/geneva/actions"
+	"github.com/getlantern/geneva/common"
+	"github.com/getlantern/geneva/internal/scanner"
 )
 
 var (
@@ -43,18 +44,10 @@ func TestIPv4HeaderChecksum(t *testing.T) {
 	}
 	expected := uint16(0xb861)
 
-	chksum := actions.ComputeIPv4Checksum(header)
+	chksum := common.CalculateIPv4Checksum(header)
 	if chksum != expected {
 		t.Fatalf("expected %#04x, got %#04x", expected, chksum)
 	}
-
-	if val := binary.BigEndian.Uint16(header[10:]); val != expected {
-		t.Fatalf("expected %#04x in header, got %#04x", expected, val)
-	}
-
-	if !actions.VerifyIPv4Checksum(header) {
-		t.Fatal("checksum validation failed")
-	}
 }
 
 func TestSendAction(t *testing.T) {

actions/actions.go

@@ -4,17 +4,21 @@
 package actions
 
 import (
+	"errors"
 	"fmt"
 
+	"github.com/google/gopacket"
+
 	"github.com/getlantern/geneva/internal"
 	"github.com/getlantern/geneva/internal/scanner"
 	"github.com/getlantern/geneva/triggers"
-	"github.com/google/gopacket"
 
 	// gopacket best practice says import this, too.
 	_ "github.com/google/gopacket/layers"
 )
 
+var ErrInvalidAction = errors.New("invalid action")
+
 // ActionTree represents a Geneva (trigger, action) pair.
 //
 // Technically, Geneva uses the term "action tree" to refer to the tree of actions in the tuple
@@ -122,5 +126,5 @@ func ParseAction(s *scanner.Scanner) (Action, error) {
 		return DefaultSendAction, nil
 	}
 
-	return nil, fmt.Errorf("invalid action at %d", s.Pos())
+	return nil, fmt.Errorf("%w at %d", ErrInvalidAction, s.Pos())
 }

actions/duplicate_action.go

@@ -4,9 +4,10 @@ import (
 	"errors"
 	"fmt"
 
+	"github.com/google/gopacket"
+
 	"github.com/getlantern/geneva/internal"
 	"github.com/getlantern/geneva/internal/scanner"
-	"github.com/google/gopacket"
 )
 
 // DuplicateAction is a Geneva action that duplicates a packet and applies separate action trees to
@@ -114,18 +115,22 @@ func ParseDuplicateAction(s *scanner.Scanner) (Action, error) {
 	}
 
 	if action.Left, err = ParseAction(s); err != nil {
+		if !errors.Is(err, ErrInvalidAction) {
+			return nil, err
+		}
+
 		if c, err2 := s.Peek(); err2 == nil && c == ',' {
 			action.Left = &SendAction{}
 		} else {
 			return nil, fmt.Errorf(
-				"error parsing first action of duplicate rule: %v",
+				"error parsing first action of duplicate rule: %w",
 				err)
 		}
 	}
 
 	if _, err = s.Expect(","); err != nil {
 		return nil, fmt.Errorf(
-			"unexpected token in duplicate rule: %v",
+			"unexpected token in duplicate rule: %w",
 			internal.EOFUnexpected(err),
 		)
 	}
@@ -135,14 +140,14 @@ func ParseDuplicateAction(s *scanner.Scanner) (Action, error) {
 			action.Right = &SendAction{}
 		} else {
 			return nil, fmt.Errorf(
-				"error parsing second action of duplicate rule: %v",
+				"error parsing second action of duplicate rule: %w",
 				err)
 		}
 	}
 
 	if _, err = s.Expect(")"); err != nil {
 		return nil, fmt.Errorf(
-			"unexpected token in duplicate rule: %v",
+			"unexpected token in duplicate rule: %w",
 			internal.EOFUnexpected(err),
 		)
 	}

actions/fragment_packet.go

@@ -7,10 +7,12 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/getlantern/geneva/internal"
-	"github.com/getlantern/geneva/internal/scanner"
 	"github.com/google/gopacket"
 	"github.com/google/gopacket/layers"
+
+	"github.com/getlantern/geneva/common"
+	"github.com/getlantern/geneva/internal"
+	"github.com/getlantern/geneva/internal/scanner"
 )
 
 // FragmentAction is a Geneva action that splits a packet into two fragments and applies separate
@@ -63,7 +65,6 @@ func (a *FragmentAction) Apply(packet gopacket.Packet) ([]gopacket.Packet, error
 	case layers.LayerTypeTCP:
 		packets, err = fragmentTCPSegment(packet, a.FragSize)
 	default:
-		// nolint: godox
 		// TODO: should we log this?
 		packets, err = duplicate(packet)
 	}
@@ -138,19 +139,12 @@ func fragmentTCPSegment(packet gopacket.Packet, fragSize int) ([]gopacket.Packet
 
 	ipv4Buf := buf[ofs:]
 
-	// fix up the IP header's Total Length field and checksum
+	// fix up the IP header's Total Length field
 	binary.BigEndian.PutUint16(ipv4Buf[2:], uint16(f1Len-ofs))
 	ipHdrLen := uint16(ipv4Buf[0]&0x0f) * 4
-	ComputeIPv4Checksum(ipv4Buf[:ipHdrLen])
-
-	chksum := ComputeTCPChecksum(
-		ipv4Buf[:ipHdrLen],
-		ipv4Buf[ipHdrLen:headersLen-ofs],
-		ipv4Buf[headersLen-ofs:],
-	)
-	binary.BigEndian.PutUint16(ipv4Buf[ipHdrLen+16:], chksum)
 
 	first := gopacket.NewPacket(buf, packet.Layers()[0].LayerType(), gopacket.NoCopy)
+	updateChecksums(first)
 
 	// create the second fragment.
 	f2Len := headersLen + tcpPayloadLen - fragSize
@@ -160,9 +154,8 @@ func fragmentTCPSegment(packet gopacket.Packet, fragSize int) ([]gopacket.Packet
 
 	ipv4Buf = buf[ofs:]
 
-	// fix up the IP header's Total Length field and checksum
+	// fix up the IP header's Total Length field
 	binary.BigEndian.PutUint16(ipv4Buf[2:], uint16(f2Len-ofs))
-	ComputeIPv4Checksum(ipv4Buf[:ipHdrLen])
 
 	// Fix up the TCP sequence number.
 	// Excitingly, Go does integer wrapping, so we don't have to.
@@ -171,15 +164,8 @@ func fragmentTCPSegment(packet gopacket.Packet, fragSize int) ([]gopacket.Packet
 	seqNum += uint32(fragSize)
 	binary.BigEndian.PutUint32(tcp[4:], seqNum)
 
-	// fix up the TCP checksum
-	chksum = ComputeTCPChecksum(
-		ipv4Buf[:ipHdrLen],
-		ipv4Buf[ipHdrLen:headersLen-ofs],
-		ipv4Buf[headersLen-ofs:],
-	)
-	binary.BigEndian.PutUint16(ipv4Buf[ipHdrLen+16:], chksum)
-
 	second := gopacket.NewPacket(buf, packet.Layers()[0].LayerType(), gopacket.NoCopy)
+	updateChecksums(second)
 
 	return []gopacket.Packet{first, second}, nil
 }
@@ -241,12 +227,13 @@ func FragmentIPPacket(packet gopacket.Packet, fragSize int) ([]gopacket.Packet,
 	flagsAndFrags := (binary.BigEndian.Uint16(ipv4Buf[6:]) | 0x20) & 0xe0
 	binary.LittleEndian.PutUint16(ipv4Buf[6:], flagsAndFrags)
 
-	ComputeIPv4Checksum(ipv4Buf[:hdrLen])
-
 	// slice off everything past the first fragment's end
 	buf = buf[:uint16(ofs)+hdrLen+offset]
 
 	first := gopacket.NewPacket(buf, packet.Layers()[0].LayerType(), gopacket.NoCopy)
+	if ipv4, ok := first.Layer(layers.LayerTypeIPv4).(*layers.IPv4); ok && ipv4 != nil {
+		common.UpdateIPv4Checksum(ipv4)
+	}
 
 	// Now start on the second fragment.
 	// First copy the old IP header as-is, then copy just the second fragment's payload right
@@ -264,84 +251,32 @@ func FragmentIPPacket(packet gopacket.Packet, fragSize int) ([]gopacket.Packet,
 	flagsAndFrags = (binary.BigEndian.Uint16(ipv4Buf[6:]) & 0x40) + uint16(fragSize)
 	binary.BigEndian.PutUint16(ipv4Buf[6:], flagsAndFrags)
 
-	ComputeIPv4Checksum(ipv4Buf[:hdrLen])
-
 	second := gopacket.NewPacket(buf, packet.Layers()[0].LayerType(), gopacket.NoCopy)
+	if ipv4, _ := second.Layer(layers.LayerTypeIPv4).(*layers.IPv4); ipv4 != nil {
+		common.UpdateIPv4Checksum(ipv4)
+	}
 
 	return []gopacket.Packet{first, second}, nil
 }
 
-// VerifyIPv4Checksum verifies whether an IPv4 header's checksum field is correct.
-func VerifyIPv4Checksum(header []byte) bool {
-	c := internal.OnesComplementChecksum{}
-
-	for i := 0; i < len(header); i += 2 {
-		c.Add(binary.BigEndian.Uint16(header[i:]))
+func updateChecksums(packet gopacket.Packet) {
+	if ipv4, _ := packet.Layer(layers.LayerTypeIPv4).(*layers.IPv4); ipv4 != nil {
+		common.UpdateIPv4Checksum(ipv4)
 	}
 
-	return c.Finalize() == 0
+	if tcp, _ := packet.Layer(layers.LayerTypeTCP).(*layers.TCP); tcp != nil {
+		common.UpdateTCPChecksum(tcp)
+	}
 }
 
-// ComputeIPv4Checksum computes a new checksum for the given IPv4 header and writes it into the
-// header.
-func ComputeIPv4Checksum(header []byte) uint16 {
-	// zero out the old checksum
-	binary.BigEndian.PutUint16(header[10:], 0)
-
+// VerifyIPv4Checksum verifies whether an IPv4 header's checksum field is correct.
+func VerifyIPv4Checksum(header []byte) bool {
 	c := internal.OnesComplementChecksum{}
 
 	for i := 0; i < len(header); i += 2 {
 		c.Add(binary.BigEndian.Uint16(header[i:]))
 	}
 
-	chksum := c.Finalize()
-	binary.BigEndian.PutUint16(header[10:], chksum)
-
-	return chksum
-}
-
-// ComputeTCPChecksum computes the checksum field of a TCP header.
-func ComputeTCPChecksum(ipHeader, tcpHeader, payload []byte) uint16 {
-	c := internal.OnesComplementChecksum{}
-
-	// pseudo-header
-	c.Add(binary.BigEndian.Uint16(ipHeader[12:])) // source ip address
-	c.Add(binary.BigEndian.Uint16(ipHeader[14:])) // destination ip address
-	c.Add(uint16(ipHeader[6]) << 8)               // protocol
-	tcpLength := binary.BigEndian.Uint16(ipHeader[2:])
-	tcpLength -= uint16((ipHeader[0] & 0xf)) * 4
-	c.Add(tcpLength) // "TCP Length" from RFC 793
-
-	// TCP header
-	for i := 0; i < len(tcpHeader); i += 2 {
-		if i == 16 {
-			// don't add existing checksum value
-			continue
-		}
-
-		c.Add(binary.BigEndian.Uint16(tcpHeader[i:]))
-	}
-
-	// TCP payload
-	for i := 0; i < len(payload); i += 2 {
-		if len(payload)-i == 1 {
-			// If there are an odd number of octets in the payload, the last octet must
-			// be padded on the right with zeros.
-			c.Add(uint16(payload[i]) << 8)
-		} else {
-			c.Add(binary.BigEndian.Uint16(payload[i:]))
-		}
-	}
-
-	return c.Finalize()
-}
-
-// VerifyTCPChecksum verifies whether a TCP header's checksum field is correct.
-func VerifyTCPChecksum(ipHeader, tcpHeader, payload []byte) bool {
-	c := internal.OnesComplementChecksum{}
-	c.Add(ComputeTCPChecksum(ipHeader, tcpHeader, payload))
-	c.Add(binary.BigEndian.Uint16(tcpHeader[16:]))
-
 	return c.Finalize() == 0
 }
 
@@ -439,6 +374,10 @@ func ParseFragmentAction(s *scanner.Scanner) (Action, error) {
 	}
 
 	if action.FirstFragmentAction, err = ParseAction(s); err != nil {
+		if !errors.Is(err, ErrInvalidAction) {
+			return nil, err
+		}
+
 		if c, err2 := s.Peek(); err2 == nil && c == ',' {
 			action.FirstFragmentAction = &SendAction{}
 		} else {
@@ -457,13 +396,13 @@ func ParseFragmentAction(s *scanner.Scanner) (Action, error) {
 		if c, err2 := s.Peek(); err2 == nil && c == ')' {
 			action.SecondFragmentAction = &SendAction{}
 		} else {
-			return nil, fmt.Errorf("error parsing second action of fragment rule: %v", err)
+			return nil, fmt.Errorf("error parsing second action of fragment rule: %w", err)
 		}
 	}
 
 	if _, err := s.Expect(")"); err != nil {
 		return nil, fmt.Errorf(
-			"unexpected token in fragment rule: %v",
+			"unexpected token in fragment rule: %w",
 			internal.EOFUnexpected(err),
 		)
 	}