Only read IPv4 ports from TCP and UDP

examples/config.yml

@@ -459,6 +459,7 @@ firewall:
   # - port: Takes `0` or `any` as any, a single number `80`, a range `200-901`, or `fragment` to match second and further fragments of fragmented packets (since there is no port available).
   #   proto: `any`, `tcp`, `udp`, or `icmp`
   #          a port specification is ignored if proto is `icmp`
+  #          only tcp and udp packets have ports, packets of any other protocol only match port `any` or `fragment`
   #   host: `any` or a literal hostname, ie `test-host`
   #   group: `any` or a literal group name, ie `default-group`
   #   groups: Same as group but accepts a list of values. Multiple values are AND'd together and a certificate would have to contain all groups to pass

outside.go

@@ -456,15 +456,27 @@ func parseV4(data []byte, incoming bool, fp *firewall.ParsedPacket) error {
 	if fp.Fragment {
 		fp.RemotePort = 0
 		fp.LocalPort = 0
-	} else if fp.Protocol == iputil.IPProtocolICMP { //note that orientation doesn't matter on ICMP
+		return nil
+	}
+
+	switch fp.Protocol {
+	case iputil.IPProtocolICMP: //note that orientation doesn't matter on ICMP
 		fp.RemotePort = binary.BigEndian.Uint16(data[ihl+4 : ihl+6]) //identifier
 		fp.LocalPort = 0                                             //code would be uint16(data[ihl+1])
-	} else if incoming {
-		fp.RemotePort = binary.BigEndian.Uint16(data[ihl : ihl+2])  //src port
-		fp.LocalPort = binary.BigEndian.Uint16(data[ihl+2 : ihl+4]) //dst port
-	} else {
-		fp.LocalPort = binary.BigEndian.Uint16(data[ihl : ihl+2])    //src port
-		fp.RemotePort = binary.BigEndian.Uint16(data[ihl+2 : ihl+4]) //dst port
+
+	case iputil.IPProtocolTCP, iputil.IPProtocolUDP:
+		if incoming {
+			fp.RemotePort = binary.BigEndian.Uint16(data[ihl : ihl+2])  //src port
+			fp.LocalPort = binary.BigEndian.Uint16(data[ihl+2 : ihl+4]) //dst port
+		} else {
+			fp.LocalPort = binary.BigEndian.Uint16(data[ihl : ihl+2])    //src port
+			fp.RemotePort = binary.BigEndian.Uint16(data[ihl+2 : ihl+4]) //dst port
+		}
+
+	default:
+		// don't set ports for protocols Nebula doesn't inspect, the first 4 bytes of GRE, IPIP, AH, etc. are not ports
+		fp.RemotePort = 0
+		fp.LocalPort = 0
 	}
 
 	return nil

outside_test.go

@@ -77,7 +77,7 @@ func Test_newPacket(t *testing.T) {
 	// account for variable ip header length - outgoing
 	h = ipv4.Header{
 		Version:  1,
-		Protocol: 2,
+		Protocol: iputil.IPProtocolUDP,
 		Len:      100,
 		Src:      net.IPv4(10, 0, 0, 1),
 		Dst:      net.IPv4(10, 0, 0, 2),
@@ -89,7 +89,7 @@ func Test_newPacket(t *testing.T) {
 	err = newPacket(b, false, p)
 
 	require.NoError(t, err)
-	assert.Equal(t, uint8(2), p.Protocol)
+	assert.Equal(t, uint8(iputil.IPProtocolUDP), p.Protocol)
 	assert.Equal(t, netip.MustParseAddr("10.0.0.1"), p.LocalAddr)
 	assert.Equal(t, netip.MustParseAddr("10.0.0.2"), p.RemoteAddr)
 	assert.Equal(t, uint16(6), p.RemotePort)
@@ -97,6 +97,34 @@ func Test_newPacket(t *testing.T) {
 	assert.False(t, p.Fragment)
 }
 
+// Test_newPacket_v4NoPorts ensures only TCP and UDP report ports. The first 4 bytes of other protocols are not ports,
+// and are often chosen by the sender: a GRE header's protocol type, an IPIP inner total length, a 6in4 flow label.
+// Reading them as ports would let a `proto: any` rule with a port match tunneled traffic.
+func Test_newPacket_v4NoPorts(t *testing.T) {
+	p := &firewall.ParsedPacket{}
+
+	for _, proto := range []uint8{4, 41, 47, 50, 51, 132} { // IPIP, 6in4, GRE, ESP, AH, SCTP
+		h := ipv4.Header{
+			Version:  4,
+			Len:      ipv4.HeaderLen,
+			Protocol: int(proto),
+			Src:      net.IPv4(10, 0, 0, 1),
+			Dst:      net.IPv4(10, 0, 0, 2),
+		}
+		b, err := h.Marshal()
+		require.NoError(t, err)
+		b = append(b, []byte{0x00, 0x00, 0x86, 0xdd}...) // a GRE header carrying IPv6 would read as port 34525
+
+		for _, incoming := range []bool{true, false} {
+			require.NoError(t, newPacket(b, incoming, p))
+			assert.Equal(t, proto, p.Protocol)
+			assert.Equal(t, uint16(0), p.RemotePort)
+			assert.Equal(t, uint16(0), p.LocalPort)
+			assert.False(t, p.Fragment)
+		}
+	}
+}
+
 func Test_newPacket_v6(t *testing.T) {
 	p := &firewall.ParsedPacket{}