diff --git a/examples/config.yml b/examples/config.yml index 20a85883..32157ab1 100644 --- a/examples/config.yml +++ b/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 diff --git a/outside.go b/outside.go index c28fe1ea..f449511e 100644 --- a/outside.go +++ b/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 diff --git a/outside_test.go b/outside_test.go index cf76fc47..ff8ed96a 100644 --- a/outside_test.go +++ b/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{}