diff --git a/examples/config.yml b/examples/config.yml index dc18aa1e..02f3ad54 100644 --- a/examples/config.yml +++ b/examples/config.yml @@ -457,10 +457,11 @@ firewall: # Rules are comprised of a protocol, port, and one or more of host, group, or CIDR # Logical evaluation is roughly: port AND proto AND (ca_sha OR ca_name) AND (host OR group OR groups OR cidr) AND (local cidr) # - 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` + # proto: `any`, `tcp`, `udp`, `udplite`, `dccp`, `sctp`, `icmp`, or any other IP protocol number from 1 to 255, ie `47` for gre # a port specification is ignored if proto is `icmp` - # Nebula only reads ports from tcp and udp, so a proto `any` rule with a specific port or range only matches tcp and udp traffic. - # Other protocols (gre, esp, sctp, etc) need port `any`. + # `icmp`, `1`, and `58` are the same, each matches both ICMP and ICMPv6 + # Nebula only reads ports from tcp, udp, udplite, dccp, and sctp, so a proto `any` rule with a specific port or range only matches those protocols. + # Rules for other protocols (gre, esp, etc) need port `any`. # 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/firewall.go b/firewall.go index 160fd504..dd7e34cb 100644 --- a/firewall.go +++ b/firewall.go @@ -89,21 +89,33 @@ type FirewallConntrack struct { // FirewallTable is the entry point for a rule, the evaluation order is: // Proto AND port AND (CA SHA or CA name) AND local CIDR AND (group OR groups OR name OR remote CIDR) type FirewallTable struct { - TCP firewallPort - UDP firewallPort - ICMP firewallPort + // Protos holds the rules for each IP protocol number, nil for protocols without rules. + // ICMP and ICMPv6 share one set of rules. + Protos [256]firewallPort AnyProto firewallPort } func newFirewallTable() *FirewallTable { return &FirewallTable{ - TCP: firewallPort{}, - UDP: firewallPort{}, - ICMP: firewallPort{}, AnyProto: firewallPort{}, } } +// protoPorts returns the rules for proto, creating them on first use. +func (ft *FirewallTable) protoPorts(proto uint8) firewallPort { + if ft.Protos[proto] == nil { + fp := firewallPort{} + ft.Protos[proto] = fp + switch proto { + case iputil.IPProtocolICMP: + ft.Protos[iputil.IPProtocolICMPv6] = fp + case iputil.IPProtocolICMPv6: + ft.Protos[iputil.IPProtocolICMP] = fp + } + } + return ft.Protos[proto] +} + type FirewallCA struct { Any *FirewallRule CANames map[string]*FirewallRule @@ -263,10 +275,6 @@ func (f *Firewall) AddRule(incoming bool, proto uint8, startPort int32, endPort } switch proto { - case iputil.IPProtocolTCP: - fp = ft.TCP - case iputil.IPProtocolUDP: - fp = ft.UDP case iputil.IPProtocolICMP, iputil.IPProtocolICMPv6: //ICMP traffic doesn't have ports, so we always coerce to "any", even if a value is provided if startPort != firewall.PortAny { @@ -274,11 +282,11 @@ func (f *Firewall) AddRule(incoming bool, proto uint8, startPort int32, endPort } startPort = firewall.PortAny endPort = firewall.PortAny - fp = ft.ICMP + fp = ft.protoPorts(proto) case firewall.ProtoAny: fp = ft.AnyProto default: - return fmt.Errorf("unknown protocol %v", proto) + fp = ft.protoPorts(proto) } // We need this rule string because we generate a hash. Removing this will break firewall reload. @@ -359,26 +367,39 @@ func AddFirewallRulesFromConfig(l *slog.Logger, inbound bool, c *config.C, fw Fi } var proto uint8 - var startPort, endPort int32 switch r.Proto { case "any": proto = firewall.ProtoAny - startPort, endPort, err = parsePort(sPort) case "tcp": proto = iputil.IPProtocolTCP - startPort, endPort, err = parsePort(sPort) case "udp": proto = iputil.IPProtocolUDP - startPort, endPort, err = parsePort(sPort) + case "udplite": + proto = iputil.IPProtocolUDPLite + case "dccp": + proto = iputil.IPProtocolDCCP + case "sctp": + proto = iputil.IPProtocolSCTP case "icmp": proto = iputil.IPProtocolICMP + default: + // Any other protocol by number. 0 is reserved for `any`. + n, perr := strconv.ParseUint(r.Proto, 10, 8) + if perr != nil || n == 0 { + return fmt.Errorf("%s rule #%v; proto was not understood; `%s`", table, i, r.Proto) + } + proto = uint8(n) + } + + var startPort, endPort int32 + if proto == iputil.IPProtocolICMP || proto == iputil.IPProtocolICMPv6 { startPort = firewall.PortAny endPort = firewall.PortAny if sPort != "" { l.Warn("ignoring port specification for ICMP firewall rule", "port", sPort) } - default: - return fmt.Errorf("%s rule #%v; proto was not understood; `%s`", table, i, r.Proto) + } else { + startPort, endPort, err = parsePort(sPort) } if err != nil { return fmt.Errorf("%s rule #%v; %s %s", table, i, errPort, err) @@ -635,22 +656,7 @@ func (ft *FirewallTable) match(p *firewall.Packet, incoming bool, c *cert.Cached return true } - switch p.Protocol { - case iputil.IPProtocolTCP: - if ft.TCP.match(p, incoming, c, caPool) { - return true - } - case iputil.IPProtocolUDP: - if ft.UDP.match(p, incoming, c, caPool) { - return true - } - case iputil.IPProtocolICMP, iputil.IPProtocolICMPv6: - if ft.ICMP.match(p, incoming, c, caPool) { - return true - } - } - - return false + return ft.Protos[p.Protocol].match(p, incoming, c, caPool) } func (fp firewallPort) addRule(f *Firewall, startPort int32, endPort int32, groups []string, host string, cidr, localCidr, caName string, caSha string) error { @@ -680,7 +686,7 @@ func (fp firewallPort) match(p *firewall.Packet, incoming bool, c *cert.CachedCe return false } - // this branch is here to catch traffic from FirewallTable.Any.match and FirewallTable.ICMP.match + // this branch is here to catch ICMP traffic from FirewallTable.AnyProto.match and the ICMP rules if p.Protocol == iputil.IPProtocolICMP || p.Protocol == iputil.IPProtocolICMPv6 { // port numbers are re-used for connection tracking of ICMP, // but we don't want to actually filter on them. diff --git a/firewall/packet.go b/firewall/packet.go index 9f2df76f..c8a10186 100644 --- a/firewall/packet.go +++ b/firewall/packet.go @@ -50,6 +50,12 @@ func (fp Packet) MarshalJSON() ([]byte, error) { proto = "icmpv6" case iputil.IPProtocolUDP: proto = "udp" + case iputil.IPProtocolUDPLite: + proto = "udplite" + case iputil.IPProtocolDCCP: + proto = "dccp" + case iputil.IPProtocolSCTP: + proto = "sctp" default: proto = fmt.Sprintf("unknown %v", fp.Protocol) } diff --git a/firewall_test.go b/firewall_test.go index eb688a09..ed4f8033 100644 --- a/firewall_test.go +++ b/firewall_test.go @@ -75,22 +75,33 @@ func TestFirewall_AddRule(t *testing.T) { require.NoError(t, fw.AddRule(true, iputil.IPProtocolTCP, 1, 1, []string{}, "", "", "", "", "")) // An empty rule is any - assert.True(t, fw.InRules.TCP[1].Any.Any.Any) - assert.Empty(t, fw.InRules.TCP[1].Any.Groups) - assert.Empty(t, fw.InRules.TCP[1].Any.Hosts) + assert.True(t, fw.InRules.Protos[iputil.IPProtocolTCP][1].Any.Any.Any) + assert.Empty(t, fw.InRules.Protos[iputil.IPProtocolTCP][1].Any.Groups) + assert.Empty(t, fw.InRules.Protos[iputil.IPProtocolTCP][1].Any.Hosts) fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c) require.NoError(t, fw.AddRule(true, iputil.IPProtocolUDP, 1, 1, []string{"g1"}, "", "", "", "", "")) - assert.Nil(t, fw.InRules.UDP[1].Any.Any) - assert.Contains(t, fw.InRules.UDP[1].Any.Groups[0].Groups, "g1") - assert.Empty(t, fw.InRules.UDP[1].Any.Hosts) + assert.Nil(t, fw.InRules.Protos[iputil.IPProtocolUDP][1].Any.Any) + assert.Contains(t, fw.InRules.Protos[iputil.IPProtocolUDP][1].Any.Groups[0].Groups, "g1") + assert.Empty(t, fw.InRules.Protos[iputil.IPProtocolUDP][1].Any.Hosts) fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c) require.NoError(t, fw.AddRule(true, iputil.IPProtocolICMP, 1, 1, []string{}, "h1", "", "", "", "")) //no matter what port is given for icmp, it should end up as "any" - assert.Nil(t, fw.InRules.ICMP[firewall.PortAny].Any.Any) - assert.Empty(t, fw.InRules.ICMP[firewall.PortAny].Any.Groups) - assert.Contains(t, fw.InRules.ICMP[firewall.PortAny].Any.Hosts, "h1") + assert.Nil(t, fw.InRules.Protos[iputil.IPProtocolICMP][firewall.PortAny].Any.Any) + assert.Empty(t, fw.InRules.Protos[iputil.IPProtocolICMP][firewall.PortAny].Any.Groups) + assert.Contains(t, fw.InRules.Protos[iputil.IPProtocolICMP][firewall.PortAny].Any.Hosts, "h1") + + // ICMP and ICMPv6 share one set of rules + require.NoError(t, fw.AddRule(true, iputil.IPProtocolICMPv6, 1, 1, []string{}, "h2", "", "", "", "")) + assert.Contains(t, fw.InRules.Protos[iputil.IPProtocolICMP][firewall.PortAny].Any.Hosts, "h2") + assert.Contains(t, fw.InRules.Protos[iputil.IPProtocolICMPv6][firewall.PortAny].Any.Hosts, "h1") + + // Any other protocol number gets its own rules + fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c) + require.NoError(t, fw.AddRule(true, 47, 0, 0, []string{"g1"}, "", "", "", "", "")) // GRE + assert.Contains(t, fw.InRules.Protos[47][firewall.PortAny].Any.Groups[0].Groups, "g1") + assert.Nil(t, fw.InRules.Protos[iputil.IPProtocolTCP]) fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c) require.NoError(t, fw.AddRule(false, firewall.ProtoAny, 1, 1, []string{}, "", ti.String(), "", "", "")) @@ -118,11 +129,11 @@ func TestFirewall_AddRule(t *testing.T) { fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c) require.NoError(t, fw.AddRule(true, iputil.IPProtocolUDP, 1, 1, []string{"g1"}, "", "", "", "ca-name", "")) - assert.Contains(t, fw.InRules.UDP[1].CANames, "ca-name") + assert.Contains(t, fw.InRules.Protos[iputil.IPProtocolUDP][1].CANames, "ca-name") fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c) require.NoError(t, fw.AddRule(true, iputil.IPProtocolUDP, 1, 1, []string{"g1"}, "", "", "", "", "ca-sha")) - assert.Contains(t, fw.InRules.UDP[1].CAShas, "ca-sha") + assert.Contains(t, fw.InRules.Protos[iputil.IPProtocolUDP][1].CAShas, "ca-sha") fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c) require.NoError(t, fw.AddRule(false, firewall.ProtoAny, 0, 0, []string{}, "any", "", "", "", "")) @@ -170,9 +181,13 @@ func TestFirewall_AddRule(t *testing.T) { require.NoError(t, fw.AddRule(false, firewall.ProtoAny, 0, 0, []string{}, "", "", "any", "", "")) assert.True(t, fw.OutRules.AnyProto[0].Any.Any.Any) + // Every protocol number is accepted + fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c) + require.NoError(t, fw.AddRule(true, math.MaxUint8, 0, 0, []string{}, "", "", "", "", "")) + assert.True(t, fw.InRules.Protos[math.MaxUint8][firewall.PortAny].Any.Any.Any) + // Test error conditions fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c) - require.Error(t, fw.AddRule(true, math.MaxUint8, 0, 0, []string{}, "", "", "", "", "")) require.Error(t, fw.AddRule(true, firewall.ProtoAny, 10, 0, []string{}, "", "", "", "", "")) } @@ -332,17 +347,16 @@ func TestFirewall_DropV6(t *testing.T) { func BenchmarkFirewallTable_match(b *testing.B) { f := &Firewall{} - ft := FirewallTable{ - TCP: firewallPort{}, - } + ft := FirewallTable{} + tcp := ft.protoPorts(iputil.IPProtocolTCP) pfix := netip.MustParsePrefix("172.1.1.1/32") - _ = ft.TCP.addRule(f, 10, 10, []string{"good-group"}, "good-host", pfix.String(), "", "", "") - _ = ft.TCP.addRule(f, 100, 100, []string{"good-group"}, "good-host", "", pfix.String(), "", "") + _ = tcp.addRule(f, 10, 10, []string{"good-group"}, "good-host", pfix.String(), "", "", "") + _ = tcp.addRule(f, 100, 100, []string{"good-group"}, "good-host", "", pfix.String(), "", "") pfix6 := netip.MustParsePrefix("fd11::11/128") - _ = ft.TCP.addRule(f, 10, 10, []string{"good-group"}, "good-host", pfix6.String(), "", "", "") - _ = ft.TCP.addRule(f, 100, 100, []string{"good-group"}, "good-host", "", pfix6.String(), "", "") + _ = tcp.addRule(f, 10, 10, []string{"good-group"}, "good-host", pfix6.String(), "", "", "") + _ = tcp.addRule(f, 100, 100, []string{"good-group"}, "good-host", "", pfix6.String(), "", "") cp := cert.NewCAPool() b.Run("fail on proto", func(b *testing.B) { @@ -872,6 +886,80 @@ func TestFirewall_ICMPPortBehavior(t *testing.T) { } +func TestFirewall_ProtocolRules(t *testing.T) { + l := test.NewLogger() + myVpnNetworksTable := new(bart.Lite) + myVpnNetworksTable.Insert(netip.MustParsePrefix("1.1.1.1/8")) + + network := netip.MustParsePrefix("1.2.3.4/24") + + c := cert.CachedCertificate{ + Certificate: &dummyCert{ + name: "host1", + networks: []netip.Prefix{network}, + groups: []string{"default-group"}, + issuer: "signer-shasum", + }, + InvertedGroups: map[string]struct{}{"default-group": {}}, + } + h := HostInfo{ + ConnectionState: &ConnectionState{ + peerCert: &c, + }, + vpnAddrs: []netip.Addr{network.Addr()}, + } + h.buildNetworks(myVpnNetworksTable, c.Certificate) + + cp := cert.NewCAPool() + + protos := []uint8{iputil.IPProtocolTCP, iputil.IPProtocolUDP, iputil.IPProtocolUDPLite, iputil.IPProtocolDCCP, iputil.IPProtocolSCTP} + for _, ruleProto := range protos { + fw := NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate) + require.NoError(t, fw.AddRule(true, ruleProto, 80, 80, []string{"any"}, "", "", "", "", "")) + + for _, pktProto := range protos { + p := firewall.Packet{ + LocalAddr: netip.MustParseAddr("1.2.3.4"), + RemoteAddr: netip.MustParseAddr("1.2.3.4"), + LocalPort: 80, + RemotePort: 5000, + Protocol: pktProto, + } + if pktProto == ruleProto { + require.NoError(t, fw.Drop(p, true, &h, cp, nil), "rule %d, packet %d", ruleProto, pktProto) + p.LocalPort = 81 + assert.Equal(t, ErrNoMatchingRule, fw.Drop(p, true, &h, cp, nil), "rule %d, packet %d, wrong port", ruleProto, pktProto) + } else { + assert.Equal(t, ErrNoMatchingRule, fw.Drop(p, true, &h, cp, nil), "rule %d, packet %d", ruleProto, pktProto) + } + } + } + + pkt := func(proto uint8) firewall.Packet { + return firewall.Packet{ + LocalAddr: netip.MustParseAddr("1.2.3.4"), + RemoteAddr: netip.MustParseAddr("1.2.3.4"), + Protocol: proto, + } + } + + // A rule on any other protocol number only matches that protocol + fw := NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate) + require.NoError(t, fw.AddRule(true, 47, firewall.PortAny, firewall.PortAny, []string{"any"}, "", "", "", "", "")) // GRE + require.NoError(t, fw.Drop(pkt(47), true, &h, cp, nil)) + assert.Equal(t, ErrNoMatchingRule, fw.Drop(pkt(50), true, &h, cp, nil)) // ESP + assert.Equal(t, ErrNoMatchingRule, fw.Drop(pkt(iputil.IPProtocolTCP), true, &h, cp, nil)) + + // A rule on either ICMP or ICMPv6 matches both + for _, ruleProto := range []uint8{iputil.IPProtocolICMP, iputil.IPProtocolICMPv6} { + fw := NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate) + require.NoError(t, fw.AddRule(true, ruleProto, firewall.PortAny, firewall.PortAny, []string{"any"}, "", "", "", "", "")) + require.NoError(t, fw.Drop(pkt(iputil.IPProtocolICMP), true, &h, cp, nil), "rule %d", ruleProto) + require.NoError(t, fw.Drop(pkt(iputil.IPProtocolICMPv6), true, &h, cp, nil), "rule %d", ruleProto) + assert.Equal(t, ErrNoMatchingRule, fw.Drop(pkt(47), true, &h, cp, nil), "rule %d", ruleProto) + } +} + func TestFirewall_DropIPSpoofing(t *testing.T) { ob := &bytes.Buffer{} l := test.NewLoggerWithOutput(ob) @@ -1327,6 +1415,15 @@ func TestAddFirewallRulesFromConfig(t *testing.T) { require.NoError(t, AddFirewallRulesFromConfig(l, false, conf, mf)) assert.Equal(t, addRuleCall{incoming: false, proto: iputil.IPProtocolUDP, startPort: 1, endPort: 1, groups: nil, host: "a", ip: "", localIp: ""}, mf.lastCall) + // Test adding udplite, dccp, and sctp rules + for name, proto := range map[string]uint8{"udplite": iputil.IPProtocolUDPLite, "dccp": iputil.IPProtocolDCCP, "sctp": iputil.IPProtocolSCTP} { + conf = config.NewC(test.NewLogger()) + mf = &mockFirewall{} + conf.Settings["firewall"] = map[string]any{"outbound": []any{map[string]any{"port": "1-2", "proto": name, "host": "a"}}} + require.NoError(t, AddFirewallRulesFromConfig(l, false, conf, mf), name) + assert.Equal(t, addRuleCall{incoming: false, proto: proto, startPort: 1, endPort: 2, groups: nil, host: "a", ip: "", localIp: ""}, mf.lastCall, name) + } + // Test adding icmp rule conf = config.NewC(test.NewLogger()) mf = &mockFirewall{} @@ -1341,6 +1438,32 @@ func TestAddFirewallRulesFromConfig(t *testing.T) { require.NoError(t, AddFirewallRulesFromConfig(l, false, conf, mf)) assert.Equal(t, addRuleCall{incoming: false, proto: iputil.IPProtocolICMP, startPort: firewall.PortAny, endPort: firewall.PortAny, groups: nil, host: "a", ip: "", localIp: ""}, mf.lastCall) + // Test adding rules by protocol number, as yaml would give a string or an int + for _, proto := range []any{"47", 47} { + conf = config.NewC(test.NewLogger()) + mf = &mockFirewall{} + conf.Settings["firewall"] = map[string]any{"outbound": []any{map[string]any{"port": "any", "proto": proto, "host": "a"}}} + require.NoError(t, AddFirewallRulesFromConfig(l, false, conf, mf), proto) + assert.Equal(t, addRuleCall{incoming: false, proto: 47, startPort: firewall.PortAny, endPort: firewall.PortAny, groups: nil, host: "a", ip: "", localIp: ""}, mf.lastCall, proto) + } + + // Test ICMP and ICMPv6 by number still ignore ports + for _, proto := range []uint8{iputil.IPProtocolICMP, iputil.IPProtocolICMPv6} { + conf = config.NewC(test.NewLogger()) + mf = &mockFirewall{} + conf.Settings["firewall"] = map[string]any{"outbound": []any{map[string]any{"port": "1", "proto": proto, "host": "a"}}} + require.NoError(t, AddFirewallRulesFromConfig(l, false, conf, mf), proto) + assert.Equal(t, addRuleCall{incoming: false, proto: proto, startPort: firewall.PortAny, endPort: firewall.PortAny, groups: nil, host: "a", ip: "", localIp: ""}, mf.lastCall, proto) + } + + // Test protocols that aren't a known name or a number from 1 to 255 + for _, proto := range []string{"0", "256", "-1", "gre"} { + conf = config.NewC(test.NewLogger()) + mf = &mockFirewall{} + conf.Settings["firewall"] = map[string]any{"outbound": []any{map[string]any{"port": "any", "proto": proto, "host": "a"}}} + require.EqualError(t, AddFirewallRulesFromConfig(l, false, conf, mf), "firewall.outbound rule #0; proto was not understood; `"+proto+"`") + } + // Test adding any rule conf = config.NewC(test.NewLogger()) mf = &mockFirewall{}