refactor checkpt

firewall/config.go

@@ -0,0 +1,333 @@
+package firewall
+
+import (
+	"errors"
+	"fmt"
+	"log/slog"
+	"net/netip"
+	"reflect"
+	"slices"
+	"strconv"
+	"strings"
+
+	"github.com/slackhq/nebula/config"
+	"github.com/slackhq/nebula/iputil"
+)
+
+// ruleAdder takes firewall rules, it lets tests see what addRulesFromConfig adds
+type ruleAdder interface {
+	AddRule(incoming bool, proto uint8, startPort int32, endPort int32, groups []string, host string, cidr, localCidr string, caName string, caSha string) error
+}
+
+// RulesFromConfig reads the inbound and outbound firewall rules from c
+func RulesFromConfig(l *slog.Logger, c *config.C) (*RulesBuilder, error) {
+	b := NewRulesBuilder(l)
+	b.defaultLocalCIDRAny = c.GetBool("firewall.default_local_cidr_any", false)
+
+	if err := addRulesFromConfig(l, false, c, b); err != nil {
+		return nil, err
+	}
+	if err := addRulesFromConfig(l, true, c, b); err != nil {
+		return nil, err
+	}
+
+	return b, nil
+}
+
+func addRulesFromConfig(l *slog.Logger, inbound bool, c *config.C, ra ruleAdder) error {
+	var table string
+	if inbound {
+		table = "firewall.inbound"
+	} else {
+		table = "firewall.outbound"
+	}
+
+	r := c.Get(table)
+	if r == nil {
+		return nil
+	}
+
+	rs, ok := r.([]any)
+	if !ok {
+		return fmt.Errorf("%s failed to parse, should be an array of rules", table)
+	}
+
+	for i, t := range rs {
+		r, err := convertRule(l, t, table, i)
+		if err != nil {
+			return fmt.Errorf("%s rule #%v; %s", table, i, err)
+		}
+
+		if r.Code != "" && r.Port != "" {
+			return fmt.Errorf("%s rule #%v; only one of port or code should be provided", table, i)
+		}
+
+		if r.Host == "" && len(r.Groups) == 0 && r.Cidr == "" && r.LocalCidr == "" && r.CAName == "" && r.CASha == "" {
+			return fmt.Errorf("%s rule #%v; at least one of host, group, cidr, local_cidr, ca_name, or ca_sha must be provided", table, i)
+		}
+
+		var sPort, errPort string
+		if r.Code != "" {
+			errPort = "code"
+			sPort = r.Code
+		} else {
+			errPort = "port"
+			sPort = r.Port
+		}
+
+		var proto uint8
+		switch r.Proto {
+		case "any":
+			proto = ProtoAny
+		case "tcp":
+			proto = iputil.IPProtocolTCP
+		case "udp":
+			proto = iputil.IPProtocolUDP
+		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 = PortAny
+			endPort = PortAny
+			if sPort != "" {
+				l.Warn("ignoring port specification for ICMP firewall rule", "port", sPort)
+			}
+		} else {
+			startPort, endPort, err = parsePort(sPort)
+		}
+		if err != nil {
+			return fmt.Errorf("%s rule #%v; %s %s", table, i, errPort, err)
+		}
+
+		if r.Cidr != "" && r.Cidr != "any" {
+			_, err = netip.ParsePrefix(r.Cidr)
+			if err != nil {
+				return fmt.Errorf("%s rule #%v; cidr did not parse; %s", table, i, err)
+			}
+		}
+
+		if r.LocalCidr != "" && r.LocalCidr != "any" {
+			_, err = netip.ParsePrefix(r.LocalCidr)
+			if err != nil {
+				return fmt.Errorf("%s rule #%v; local_cidr did not parse; %s", table, i, err)
+			}
+		}
+
+		if warning := r.sanity(); warning != nil {
+			l.Warn("firewall rule sanity check",
+				"table", table,
+				"rule", i,
+				"warning", warning,
+			)
+		}
+
+		err = ra.AddRule(inbound, proto, startPort, endPort, r.Groups, r.Host, r.Cidr, r.LocalCidr, r.CAName, r.CASha)
+		if err != nil {
+			return fmt.Errorf("%s rule #%v; `%s`", table, i, err)
+		}
+	}
+
+	return nil
+}
+
+type configRule struct {
+	Port      string
+	Code      string
+	Proto     string
+	Host      string
+	Groups    []string
+	Cidr      string
+	LocalCidr string
+	CAName    string
+	CASha     string
+}
+
+func convertRule(l *slog.Logger, p any, table string, i int) (configRule, error) {
+	r := configRule{}
+
+	m, ok := p.(map[string]any)
+	if !ok {
+		return r, errors.New("could not parse rule")
+	}
+
+	toString := func(k string, m map[string]any) string {
+		v, ok := m[k]
+		if !ok {
+			return ""
+		}
+		return fmt.Sprintf("%v", v)
+	}
+
+	r.Port = toString("port", m)
+	r.Code = toString("code", m)
+	r.Proto = toString("proto", m)
+	r.Host = toString("host", m)
+	r.Cidr = toString("cidr", m)
+	r.LocalCidr = toString("local_cidr", m)
+	r.CAName = toString("ca_name", m)
+	r.CASha = toString("ca_sha", m)
+
+	// Make sure group isn't an array
+	if v, ok := m["group"].([]any); ok {
+		if len(v) > 1 {
+			return r, errors.New("group should contain a single value, an array with more than one entry was provided")
+		}
+
+		l.Warn("group was an array with a single value, converting to simple value",
+			"table", table,
+			"rule", i,
+		)
+		m["group"] = v[0]
+	}
+
+	singleGroup := toString("group", m)
+
+	if rg, ok := m["groups"]; ok {
+		switch reflect.TypeOf(rg).Kind() {
+		case reflect.Slice:
+			v := reflect.ValueOf(rg)
+			r.Groups = make([]string, v.Len())
+			for i := 0; i < v.Len(); i++ {
+				r.Groups[i] = v.Index(i).Interface().(string)
+			}
+		case reflect.String:
+			r.Groups = []string{rg.(string)}
+		default:
+			r.Groups = []string{fmt.Sprintf("%v", rg)}
+		}
+	}
+
+	//flatten group vs groups
+	if singleGroup != "" {
+		// Check if we have both groups and group provided in the rule config
+		if len(r.Groups) > 0 {
+			return r, fmt.Errorf("only one of group or groups should be defined, both provided")
+		}
+		r.Groups = []string{singleGroup}
+	}
+
+	return r, nil
+}
+
+// sanity returns an error if the rule would be evaluated in a way that would short-circuit a configured check on a wildcard value
+// rules are evaluated as "port AND proto AND (ca_sha OR ca_name) AND (host OR group OR groups OR cidr) AND local_cidr"
+func (r *configRule) sanity() error {
+	//port, proto, local_cidr are AND, no need to check here
+	//ca_sha and ca_name don't have a wildcard value, no need to check here
+	groupsEmpty := len(r.Groups) == 0
+	hostEmpty := r.Host == ""
+	cidrEmpty := r.Cidr == ""
+
+	if (groupsEmpty && hostEmpty && cidrEmpty) == true {
+		return nil //no content!
+	}
+
+	groupsHasAny := slices.Contains(r.Groups, "any")
+	if groupsHasAny && len(r.Groups) > 1 {
+		return fmt.Errorf("groups spec [%s] contains the group '\"any\". This rule will ignore the other groups specified", r.Groups)
+	}
+
+	if r.Host == "any" {
+		if !groupsEmpty {
+			return fmt.Errorf("groups specified as %s, but host=any will match any host, regardless of groups", r.Groups)
+		}
+
+		if !cidrEmpty {
+			return fmt.Errorf("cidr specified as %s, but host=any will match any host, regardless of cidr", r.Cidr)
+		}
+	}
+
+	if groupsHasAny {
+		if !hostEmpty && r.Host != "any" {
+			return fmt.Errorf("groups spec [%s] contains the group '\"any\". This rule will ignore the specified host %s", r.Groups, r.Host)
+		}
+		if !cidrEmpty {
+			return fmt.Errorf("groups spec [%s] contains the group '\"any\". This rule will ignore the specified cidr %s", r.Groups, r.Cidr)
+		}
+	}
+
+	if r.Code != "" {
+		return fmt.Errorf("code specified as [%s]. Support for 'code' will be dropped in a future release, as it has never been functional", r.Code)
+	}
+
+	//todo alert on cidr-any
+
+	return nil
+}
+
+func parsePort(s string) (int32, int32, error) {
+	const notAPort int32 = -2
+	if s == "any" {
+		return PortAny, PortAny, nil
+	}
+	if s == "fragment" {
+		return PortFragment, PortFragment, nil
+	}
+	if !strings.Contains(s, `-`) {
+		rPort, err := parsePortValue("", s)
+		if err != nil {
+			return notAPort, notAPort, err
+		}
+		return rPort, rPort, nil
+	}
+
+	sPorts := strings.SplitN(s, `-`, 2)
+	for i := range sPorts {
+		sPorts[i] = strings.Trim(sPorts[i], " ")
+	}
+	if len(sPorts) != 2 || sPorts[0] == "" || sPorts[1] == "" {
+		return notAPort, notAPort, fmt.Errorf("appears to be a range but could not be parsed; `%s`", s)
+	}
+
+	startPort, err := parsePortValue("beginning range ", sPorts[0])
+	if err != nil {
+		return notAPort, notAPort, err
+	}
+
+	endPort, err := parsePortValue("ending range ", sPorts[1])
+	if err != nil {
+		return notAPort, notAPort, err
+	}
+
+	if startPort == PortAny {
+		endPort = PortAny
+	}
+
+	return startPort, endPort, nil
+}
+
+// parsePortValue accepts a base-10 decimal in [0, 65535] and returns it
+// widened to int32. Using strconv.ParseUint with bitSize 16 rejects
+// negative input, out-of-range input (>65535), and any non-decimal byte
+// by construction, so the int32 widening that follows is provably safe
+// and cannot collide with PortAny (0) or PortFragment
+// (-1) via integer truncation.
+//
+// prefix is prepended to both error messages so callers can disambiguate
+// the single-port path (prefix="") from the range bounds (prefix="beginning
+// range " / "ending range "), preserving the historical error strings.
+func parsePortValue(prefix, s string) (int32, error) {
+	n, err := strconv.ParseUint(s, 10, 16)
+	if err == nil {
+		return int32(n), nil
+	}
+	if errors.Is(err, strconv.ErrRange) {
+		return 0, fmt.Errorf("%sout of range [0,65535]; `%s`", prefix, s)
+	}
+	return 0, fmt.Errorf("%swas not a number; `%s`", prefix, s)
+}

firewall/config_test.go

@@ -0,0 +1,423 @@
+package firewall
+
+import (
+	"bytes"
+	"errors"
+	"net/netip"
+	"testing"
+
+	"github.com/slackhq/nebula/config"
+	"github.com/slackhq/nebula/iputil"
+	"github.com/slackhq/nebula/test"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func Test_parsePort(t *testing.T) {
+	_, _, err := parsePort("")
+	require.EqualError(t, err, "was not a number; ``")
+
+	_, _, err = parsePort("  ")
+	require.EqualError(t, err, "was not a number; `  `")
+
+	_, _, err = parsePort("-")
+	require.EqualError(t, err, "appears to be a range but could not be parsed; `-`")
+
+	_, _, err = parsePort(" - ")
+	require.EqualError(t, err, "appears to be a range but could not be parsed; ` - `")
+
+	_, _, err = parsePort("a-b")
+	require.EqualError(t, err, "beginning range was not a number; `a`")
+
+	_, _, err = parsePort("1-b")
+	require.EqualError(t, err, "ending range was not a number; `b`")
+
+	s, e, err := parsePort(" 1 - 2    ")
+	assert.Equal(t, int32(1), s)
+	assert.Equal(t, int32(2), e)
+	require.NoError(t, err)
+
+	s, e, err = parsePort("0-1")
+	assert.Equal(t, int32(0), s)
+	assert.Equal(t, int32(0), e)
+	require.NoError(t, err)
+
+	s, e, err = parsePort("9919")
+	assert.Equal(t, int32(9919), s)
+	assert.Equal(t, int32(9919), e)
+	require.NoError(t, err)
+
+	s, e, err = parsePort("any")
+	assert.Equal(t, int32(0), s)
+	assert.Equal(t, int32(0), e)
+	require.NoError(t, err)
+}
+
+// Test_parsePort_invalid covers inputs that must error. The named bug is
+// that int32(strconv.Atoi("4294967296")) truncates to 0 == PortAny,
+// silently turning a typo into a match-all-ports rule; the rest are
+// representative syntax/range probes.
+func Test_parsePort_invalid(t *testing.T) {
+	tests := []struct {
+		name            string
+		input           string
+		wantErrContains string
+	}{
+		// Numeric overflow (the named bug + boundary).
+		{"named bug: 2^32 truncates to PortAny", "4294967296", "out of range"},
+		{"just above max real port", "65536", "out of range"},
+
+		// Negatives route through the range branch and hit the empty-half
+		// guard; included as defense in depth so a future refactor cannot
+		// accidentally reach the int32 cast.
+		{"negative", "-1", "could not be parsed"},
+
+		// Syntax probes.
+		{"NUL between digits", "4\x002", "was not a number"},
+		{"hex notation", "0x10", "was not a number"},
+		{"scientific notation", "1e3", "was not a number"},
+		{"leading whitespace", " 42", "was not a number"},
+		{"fullwidth digits", "４２", "was not a number"},
+
+		// Range branch.
+		{"range upper out of range", "1-65536", "ending range out of range"},
+		{"range lower out of range", "65536-65537", "beginning range out of range"},
+		{"range with negative upper", "1--1", "ending range was not a number"},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			_, _, err := parsePort(tc.input)
+			require.Error(t, err, "input %q must error", tc.input)
+			require.ErrorContains(t, err, tc.wantErrContains)
+		})
+	}
+}
+
+// Test_parsePort_valid_boundaries locks in success cases at 0, 1, and 65535
+// so a future refactor cannot regress the boundaries.
+func Test_parsePort_valid_boundaries(t *testing.T) {
+	tests := []struct {
+		name      string
+		input     string
+		wantStart int32
+		wantEnd   int32
+	}{
+		{"zero is PortAny", "0", 0, 0},
+		{"min real port", "1", 1, 1},
+		{"max real port", "65535", 65535, 65535},
+		{"range zero to max forces end to zero", "0-65535", 0, 0},
+		{"range max to max", "65535-65535", 65535, 65535},
+		{"range one to max", "1-65535", 1, 65535},
+		{"range with whitespace inside", " 1 - 2    ", 1, 2},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			s, e, err := parsePort(tc.input)
+			require.NoError(t, err)
+			assert.Equal(t, tc.wantStart, s, "start port")
+			assert.Equal(t, tc.wantEnd, e, "end port")
+		})
+	}
+}
+
+func TestAddRulesFromConfig(t *testing.T) {
+	l := test.NewLogger()
+	// Test adding tcp rule
+	conf := config.NewC(test.NewLogger())
+	mf := &mockFirewall{}
+	conf.Settings["firewall"] = map[string]any{"outbound": []any{map[string]any{"port": "1", "proto": "tcp", "host": "a"}}}
+	require.NoError(t, addRulesFromConfig(l, false, conf, mf))
+	assert.Equal(t, addRuleCall{incoming: false, proto: iputil.IPProtocolTCP, startPort: 1, endPort: 1, groups: nil, host: "a", ip: "", localIp: ""}, mf.lastCall)
+
+	// Test adding udp rule
+	conf = config.NewC(test.NewLogger())
+	mf = &mockFirewall{}
+	conf.Settings["firewall"] = map[string]any{"outbound": []any{map[string]any{"port": "1", "proto": "udp", "host": "a"}}}
+	require.NoError(t, addRulesFromConfig(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, addRulesFromConfig(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{}
+	conf.Settings["firewall"] = map[string]any{"outbound": []any{map[string]any{"port": "1", "proto": "icmp", "host": "a"}}}
+	require.NoError(t, addRulesFromConfig(l, false, conf, mf))
+	assert.Equal(t, addRuleCall{incoming: false, proto: iputil.IPProtocolICMP, startPort: PortAny, endPort: PortAny, groups: nil, host: "a", ip: "", localIp: ""}, mf.lastCall)
+
+	// Test adding icmp rule no port
+	conf = config.NewC(test.NewLogger())
+	mf = &mockFirewall{}
+	conf.Settings["firewall"] = map[string]any{"outbound": []any{map[string]any{"proto": "icmp", "host": "a"}}}
+	require.NoError(t, addRulesFromConfig(l, false, conf, mf))
+	assert.Equal(t, addRuleCall{incoming: false, proto: iputil.IPProtocolICMP, startPort: PortAny, endPort: 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, addRulesFromConfig(l, false, conf, mf), proto)
+		assert.Equal(t, addRuleCall{incoming: false, proto: 47, startPort: PortAny, endPort: 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, addRulesFromConfig(l, false, conf, mf), proto)
+		assert.Equal(t, addRuleCall{incoming: false, proto: proto, startPort: PortAny, endPort: 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, addRulesFromConfig(l, false, conf, mf), "firewall.outbound rule #0; proto was not understood; `"+proto+"`")
+	}
+
+	// Test adding any rule
+	conf = config.NewC(test.NewLogger())
+	mf = &mockFirewall{}
+	conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "host": "a"}}}
+	require.NoError(t, addRulesFromConfig(l, true, conf, mf))
+	assert.Equal(t, addRuleCall{incoming: true, proto: ProtoAny, startPort: 1, endPort: 1, groups: nil, host: "a", ip: "", localIp: ""}, mf.lastCall)
+
+	// Test adding rule with cidr
+	cidr := netip.MustParsePrefix("10.0.0.0/8")
+	conf = config.NewC(test.NewLogger())
+	mf = &mockFirewall{}
+	conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "cidr": cidr.String()}}}
+	require.NoError(t, addRulesFromConfig(l, true, conf, mf))
+	assert.Equal(t, addRuleCall{incoming: true, proto: ProtoAny, startPort: 1, endPort: 1, groups: nil, ip: cidr.String(), localIp: ""}, mf.lastCall)
+
+	// Test adding rule with local_cidr
+	conf = config.NewC(test.NewLogger())
+	mf = &mockFirewall{}
+	conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "local_cidr": cidr.String()}}}
+	require.NoError(t, addRulesFromConfig(l, true, conf, mf))
+	assert.Equal(t, addRuleCall{incoming: true, proto: ProtoAny, startPort: 1, endPort: 1, groups: nil, ip: "", localIp: cidr.String()}, mf.lastCall)
+
+	// Test adding rule with cidr ipv6
+	cidr6 := netip.MustParsePrefix("fd00::/8")
+	conf = config.NewC(test.NewLogger())
+	mf = &mockFirewall{}
+	conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "cidr": cidr6.String()}}}
+	require.NoError(t, addRulesFromConfig(l, true, conf, mf))
+	assert.Equal(t, addRuleCall{incoming: true, proto: ProtoAny, startPort: 1, endPort: 1, groups: nil, ip: cidr6.String(), localIp: ""}, mf.lastCall)
+
+	// Test adding rule with any cidr
+	conf = config.NewC(test.NewLogger())
+	mf = &mockFirewall{}
+	conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "cidr": "any"}}}
+	require.NoError(t, addRulesFromConfig(l, true, conf, mf))
+	assert.Equal(t, addRuleCall{incoming: true, proto: ProtoAny, startPort: 1, endPort: 1, groups: nil, ip: "any", localIp: ""}, mf.lastCall)
+
+	// Test adding rule with junk cidr
+	conf = config.NewC(test.NewLogger())
+	mf = &mockFirewall{}
+	conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "cidr": "junk/junk"}}}
+	require.EqualError(t, addRulesFromConfig(l, true, conf, mf), "firewall.inbound rule #0; cidr did not parse; netip.ParsePrefix(\"junk/junk\"): ParseAddr(\"junk\"): unable to parse IP")
+
+	// Test adding rule with local_cidr ipv6
+	conf = config.NewC(test.NewLogger())
+	mf = &mockFirewall{}
+	conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "local_cidr": cidr6.String()}}}
+	require.NoError(t, addRulesFromConfig(l, true, conf, mf))
+	assert.Equal(t, addRuleCall{incoming: true, proto: ProtoAny, startPort: 1, endPort: 1, groups: nil, ip: "", localIp: cidr6.String()}, mf.lastCall)
+
+	// Test adding rule with any local_cidr
+	conf = config.NewC(test.NewLogger())
+	mf = &mockFirewall{}
+	conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "local_cidr": "any"}}}
+	require.NoError(t, addRulesFromConfig(l, true, conf, mf))
+	assert.Equal(t, addRuleCall{incoming: true, proto: ProtoAny, startPort: 1, endPort: 1, groups: nil, localIp: "any"}, mf.lastCall)
+
+	// Test adding rule with junk local_cidr
+	conf = config.NewC(test.NewLogger())
+	mf = &mockFirewall{}
+	conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "local_cidr": "junk/junk"}}}
+	require.EqualError(t, addRulesFromConfig(l, true, conf, mf), "firewall.inbound rule #0; local_cidr did not parse; netip.ParsePrefix(\"junk/junk\"): ParseAddr(\"junk\"): unable to parse IP")
+
+	// Test adding rule with ca_sha
+	conf = config.NewC(test.NewLogger())
+	mf = &mockFirewall{}
+	conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "ca_sha": "12312313123"}}}
+	require.NoError(t, addRulesFromConfig(l, true, conf, mf))
+	assert.Equal(t, addRuleCall{incoming: true, proto: ProtoAny, startPort: 1, endPort: 1, groups: nil, ip: "", localIp: "", caSha: "12312313123"}, mf.lastCall)
+
+	// Test adding rule with ca_name
+	conf = config.NewC(test.NewLogger())
+	mf = &mockFirewall{}
+	conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "ca_name": "root01"}}}
+	require.NoError(t, addRulesFromConfig(l, true, conf, mf))
+	assert.Equal(t, addRuleCall{incoming: true, proto: ProtoAny, startPort: 1, endPort: 1, groups: nil, ip: "", localIp: "", caName: "root01"}, mf.lastCall)
+
+	// Test single group
+	conf = config.NewC(test.NewLogger())
+	mf = &mockFirewall{}
+	conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "group": "a"}}}
+	require.NoError(t, addRulesFromConfig(l, true, conf, mf))
+	assert.Equal(t, addRuleCall{incoming: true, proto: ProtoAny, startPort: 1, endPort: 1, groups: []string{"a"}, ip: "", localIp: ""}, mf.lastCall)
+
+	// Test single groups
+	conf = config.NewC(test.NewLogger())
+	mf = &mockFirewall{}
+	conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "groups": "a"}}}
+	require.NoError(t, addRulesFromConfig(l, true, conf, mf))
+	assert.Equal(t, addRuleCall{incoming: true, proto: ProtoAny, startPort: 1, endPort: 1, groups: []string{"a"}, ip: "", localIp: ""}, mf.lastCall)
+
+	// Test multiple AND groups
+	conf = config.NewC(test.NewLogger())
+	mf = &mockFirewall{}
+	conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "groups": []string{"a", "b"}}}}
+	require.NoError(t, addRulesFromConfig(l, true, conf, mf))
+	assert.Equal(t, addRuleCall{incoming: true, proto: ProtoAny, startPort: 1, endPort: 1, groups: []string{"a", "b"}, ip: "", localIp: ""}, mf.lastCall)
+
+	// Test Add error
+	conf = config.NewC(test.NewLogger())
+	mf = &mockFirewall{}
+	mf.nextCallReturn = errors.New("test error")
+	conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "host": "a"}}}
+	require.EqualError(t, addRulesFromConfig(l, true, conf, mf), "firewall.inbound rule #0; `test error`")
+}
+
+func Test_convertRule(t *testing.T) {
+	ob := &bytes.Buffer{}
+	l := test.NewLoggerWithOutput(ob)
+
+	// Ensure group array of 1 is converted and a warning is printed
+	c := map[string]any{
+		"group": []any{"group1"},
+	}
+
+	r, err := convertRule(l, c, "test", 1)
+	assert.Contains(t, ob.String(), "group was an array with a single value, converting to simple value")
+	assert.Contains(t, ob.String(), "table=test")
+	assert.Contains(t, ob.String(), "rule=1")
+	require.NoError(t, err)
+	assert.Equal(t, []string{"group1"}, r.Groups)
+
+	// Ensure group array of > 1 is errord
+	ob.Reset()
+	c = map[string]any{
+		"group": []any{"group1", "group2"},
+	}
+
+	r, err = convertRule(l, c, "test", 1)
+	assert.Empty(t, ob.String())
+	require.Error(t, err, "group should contain a single value, an array with more than one entry was provided")
+
+	// Make sure a well formed group is alright
+	ob.Reset()
+	c = map[string]any{
+		"group": "group1",
+	}
+
+	r, err = convertRule(l, c, "test", 1)
+	require.NoError(t, err)
+	assert.Equal(t, []string{"group1"}, r.Groups)
+}
+
+func Test_convertRuleSanity(t *testing.T) {
+	ob := &bytes.Buffer{}
+	l := test.NewLoggerWithOutput(ob)
+
+	noWarningPlease := []map[string]any{
+		{"group": "group1"},
+		{"groups": []any{"group2"}},
+		{"host": "bob"},
+		{"cidr": "1.1.1.1/1"},
+		{"groups": []any{"group2"}, "host": "bob"},
+		{"cidr": "1.1.1.1/1", "host": "bob"},
+		{"groups": []any{"group2"}, "cidr": "1.1.1.1/1"},
+		{"groups": []any{"group2"}, "cidr": "1.1.1.1/1", "host": "bob"},
+	}
+	for _, c := range noWarningPlease {
+		r, err := convertRule(l, c, "test", 1)
+		require.NoError(t, err)
+		require.NoError(t, r.sanity(), "should not generate a sanity warning, %+v", c)
+	}
+
+	yesWarningPlease := []map[string]any{
+		{"group": "group1"},
+		{"groups": []any{"group2"}},
+		{"cidr": "1.1.1.1/1"},
+		{"groups": []any{"group2"}, "host": "bob"},
+		{"cidr": "1.1.1.1/1", "host": "bob"},
+		{"groups": []any{"group2"}, "cidr": "1.1.1.1/1"},
+		{"groups": []any{"group2"}, "cidr": "1.1.1.1/1", "host": "bob"},
+	}
+	for _, c := range yesWarningPlease {
+		c["host"] = "any"
+		r, err := convertRule(l, c, "test", 1)
+		require.NoError(t, err)
+		err = r.sanity()
+		require.Error(t, err, "I wanted a warning: %+v", c)
+	}
+	//reset the list
+	yesWarningPlease = []map[string]any{
+		{"group": "group1"},
+		{"groups": []any{"group2"}},
+		{"cidr": "1.1.1.1/1"},
+		{"groups": []any{"group2"}, "host": "bob"},
+		{"cidr": "1.1.1.1/1", "host": "bob"},
+		{"groups": []any{"group2"}, "cidr": "1.1.1.1/1"},
+		{"groups": []any{"group2"}, "cidr": "1.1.1.1/1", "host": "bob"},
+	}
+	for _, c := range yesWarningPlease {
+		r, err := convertRule(l, c, "test", 1)
+		require.NoError(t, err)
+		r.Groups = append(r.Groups, "any")
+		err = r.sanity()
+		require.Error(t, err, "I wanted a warning: %+v", c)
+	}
+}
+
+type addRuleCall struct {
+	incoming  bool
+	proto     uint8
+	startPort int32
+	endPort   int32
+	groups    []string
+	host      string
+	ip        string
+	localIp   string
+	caName    string
+	caSha     string
+}
+
+type mockFirewall struct {
+	lastCall       addRuleCall
+	nextCallReturn error
+}
+
+func (mf *mockFirewall) AddRule(incoming bool, proto uint8, startPort int32, endPort int32, groups []string, host string, ip, localIp, caName string, caSha string) error {
+	mf.lastCall = addRuleCall{
+		incoming:  incoming,
+		proto:     proto,
+		startPort: startPort,
+		endPort:   endPort,
+		groups:    groups,
+		host:      host,
+		ip:        ip,
+		localIp:   localIp,
+		caName:    caName,
+		caSha:     caSha,
+	}
+
+	err := mf.nextCallReturn
+	mf.nextCallReturn = nil
+	return err
+}

firewall/rules_builder.go

@@ -0,0 +1,328 @@
+package firewall
+
+import (
+	"crypto/sha256"
+	"encoding/hex"
+	"fmt"
+	"hash"
+	"hash/fnv"
+	"io"
+	"log/slog"
+	"net/netip"
+	"slices"
+
+	"github.com/gaissmai/bart"
+	"github.com/slackhq/nebula/iputil"
+)
+
+// RulesBuilder collects the inbound and outbound rules, Build turns them into a Table for each direction
+type RulesBuilder struct {
+	in, out tableRules
+
+	// defaultLocalCIDRAny makes rules without a local cidr allow any local address, even with unsafe networks
+	defaultLocalCIDRAny bool
+
+	// ruleSHA and ruleFNV hash every rule as it's added, see Rules
+	ruleSHA hash.Hash
+	ruleFNV hash.Hash32
+
+	l *slog.Logger
+}
+
+// Rules are the Tables built for each direction, with hashes of the rules they were built from
+type Rules struct {
+	In, Out *Table
+
+	// Hash is a sha256 of the rules, HashFNV is an FNV-1a of them for use as a metric value.
+	// Rule sets with the same rules have the same hashes.
+	Hash    string
+	HashFNV uint32
+}
+
+// tableRules are one direction's rules, waiting to be built into a Table
+type tableRules struct {
+	anyRules []rule
+	// protoRules holds the rules for each protocol besides proto `any`, ICMPv6 rules are kept with ICMP
+	protoRules [256][]rule
+}
+
+// rule is a parsed rule, so adding it to a Table can't fail
+type rule struct {
+	startPort, endPort int32
+	groups             []string
+	host               string
+	caName, caSha      string
+
+	// cidr is the remote cidr, not valid when the rule doesn't have one
+	cidr    netip.Prefix
+	anyCIDR bool
+
+	// localCIDRs are the local addresses the rule allows, anyLocalCIDR allows all of them.
+	// Neither is set for a rule without a local cidr until the Table is built and it gets the default.
+	localCIDRs   []netip.Prefix
+	anyLocalCIDR bool
+}
+
+func NewRulesBuilder(l *slog.Logger) *RulesBuilder {
+	return &RulesBuilder{
+		ruleSHA: sha256.New(),
+		ruleFNV: fnv.New32a(),
+		l:       l,
+	}
+}
+
+// AddRule adds a rule for incoming or outgoing traffic, on proto or on every protocol when proto is ProtoAny.
+// The rule is parsed now, so a bad rule is reported when it's added rather than when the Tables are built.
+func (b *RulesBuilder) AddRule(incoming bool, proto uint8, startPort int32, endPort int32, groups []string, host string, cidr, localCidr, caName string, caSha string) error {
+	if proto == iputil.IPProtocolICMP || proto == iputil.IPProtocolICMPv6 {
+		//ICMP traffic doesn't have ports, so we always coerce to "any", even if a value is provided
+		if startPort != PortAny {
+			b.l.Warn("ignoring port specification for ICMP firewall rule", "startPort", startPort)
+		}
+		startPort = PortAny
+		endPort = PortAny
+	}
+
+	r, err := parseRule(startPort, endPort, groups, host, cidr, localCidr, caName, caSha)
+	if err != nil {
+		return err
+	}
+
+	if incoming {
+		b.in.add(proto, r)
+	} else {
+		b.out.add(proto, r)
+	}
+
+	// The rule hashes are of these lines, changing them changes every hash
+	fmt.Fprintf(io.MultiWriter(b.ruleSHA, b.ruleFNV),
+		"incoming: %v, proto: %v, startPort: %v, endPort: %v, groups: %v, host: %v, ip: %v, localIp: %v, caName: %v, caSha: %s\n",
+		incoming, proto, startPort, endPort, groups, host, cidr, localCidr, caName, caSha,
+	)
+
+	direction := "incoming"
+	if !incoming {
+		direction = "outgoing"
+	}
+	b.l.Info("Firewall rule added",
+		"firewallRule", m{"direction": direction, "proto": proto, "startPort": startPort, "endPort": endPort, "groups": groups, "host": host, "cidr": cidr, "localCidr": localCidr, "caName": caName, "caSha": caSha},
+	)
+
+	return nil
+}
+
+// Build turns the rules into a Table for each direction, and hashes them. Rules without a local cidr allow
+// vpnNetworks when there are unsafe networks, otherwise any local address.
+func (b *RulesBuilder) Build(vpnNetworks, unsafeNetworks []netip.Prefix) Rules {
+	anyLocal := len(unsafeNetworks) == 0 || b.defaultLocalCIDRAny
+	return Rules{
+		In:      b.in.build(anyLocal, vpnNetworks),
+		Out:     b.out.build(anyLocal, vpnNetworks),
+		Hash:    hex.EncodeToString(b.ruleSHA.Sum(nil)),
+		HashFNV: b.ruleFNV.Sum32(),
+	}
+}
+
+func parseRule(startPort, endPort int32, groups []string, host, cidr, localCidr, caName, caSha string) (rule, error) {
+	if startPort > endPort {
+		return rule{}, fmt.Errorf("start port was lower than end port")
+	}
+
+	r := rule{
+		startPort: startPort,
+		endPort:   endPort,
+		groups:    groups,
+		host:      host,
+		caName:    caName,
+		caSha:     caSha,
+	}
+
+	var err error
+	r.cidr, r.anyCIDR, err = parseCIDR(cidr)
+	if err != nil {
+		return rule{}, err
+	}
+
+	var localCIDR netip.Prefix
+	localCIDR, r.anyLocalCIDR, err = parseCIDR(localCidr)
+	if err != nil {
+		return rule{}, err
+	}
+	if localCIDR.IsValid() {
+		r.localCIDRs = []netip.Prefix{localCIDR}
+	}
+
+	return r, nil
+}
+
+// add keeps r for proto, or for every protocol when proto is ProtoAny
+func (tr *tableRules) add(proto uint8, r rule) {
+	switch proto {
+	case ProtoAny:
+		tr.anyRules = append(tr.anyRules, r)
+	case iputil.IPProtocolICMPv6:
+		tr.protoRules[iputil.IPProtocolICMP] = append(tr.protoRules[iputil.IPProtocolICMP], r)
+	default:
+		tr.protoRules[proto] = append(tr.protoRules[proto], r)
+	}
+}
+
+// build turns the rules into a Table. Rules without a local cidr allow defaultLocalCIDRs,
+// or any local address when defaultLocalAny is set.
+func (tr *tableRules) build(defaultLocalAny bool, defaultLocalCIDRs []netip.Prefix) *Table {
+	add := func(pr portRules, rules []rule) {
+		for _, r := range rules {
+			if !r.anyLocalCIDR && r.localCIDRs == nil {
+				r.anyLocalCIDR, r.localCIDRs = defaultLocalAny, defaultLocalCIDRs
+			}
+			pr.add(r)
+		}
+	}
+
+	var anyProto portRules
+	if len(tr.anyRules) > 0 {
+		anyProto = portRules{}
+		add(anyProto, tr.anyRules)
+	}
+
+	t := &Table{}
+	for proto := range t.Protos {
+		rules := tr.protoRules[proto]
+		if len(rules) == 0 {
+			t.Protos[proto] = anyProto
+			continue
+		}
+
+		pr := portRules{}
+		add(pr, tr.anyRules)
+		add(pr, rules)
+		t.Protos[proto] = pr
+	}
+	t.Protos[iputil.IPProtocolICMPv6] = t.Protos[iputil.IPProtocolICMP]
+
+	return t
+}
+
+// parseCIDR parses a rule's cidr, which is empty, `any`, or a prefix
+func parseCIDR(s string) (prefix netip.Prefix, isAny bool, err error) {
+	switch s {
+	case "":
+		return netip.Prefix{}, false, nil
+	case "any":
+		return netip.Prefix{}, true, nil
+	}
+
+	prefix, err = netip.ParsePrefix(s)
+	return prefix, false, err
+}
+
+// isAny reports whether r matches any host, making its groups, host, and cidr irrelevant
+func (r rule) isAny() bool {
+	if len(r.groups) == 0 && r.host == "" && !r.cidr.IsValid() {
+		return true
+	}
+
+	return slices.Contains(r.groups, "any") || r.host == "any" || r.anyCIDR
+}
+
+func (pr portRules) add(r rule) {
+	for i := r.startPort; i <= r.endPort; i++ {
+		if _, ok := pr[i]; !ok {
+			pr[i] = &caRules{
+				CANames: make(map[string]*remoteRules),
+				CAShas:  make(map[string]*remoteRules),
+			}
+		}
+
+		pr[i].add(r)
+	}
+}
+
+func (cr *caRules) add(r rule) {
+	newRemoteRules := func() *remoteRules {
+		return &remoteRules{
+			Hosts:  make(map[string]*localRules),
+			Groups: make([]*groupsRule, 0),
+			CIDR:   new(bart.Table[*localRules]),
+		}
+	}
+
+	if r.caSha == "" && r.caName == "" {
+		if cr.Any == nil {
+			cr.Any = newRemoteRules()
+		}
+
+		cr.Any.add(r)
+		return
+	}
+
+	if r.caSha != "" {
+		if _, ok := cr.CAShas[r.caSha]; !ok {
+			cr.CAShas[r.caSha] = newRemoteRules()
+		}
+		cr.CAShas[r.caSha].add(r)
+	}
+
+	if r.caName != "" {
+		if _, ok := cr.CANames[r.caName]; !ok {
+			cr.CANames[r.caName] = newRemoteRules()
+		}
+		cr.CANames[r.caName].add(r)
+	}
+}
+
+func (rr *remoteRules) add(r rule) {
+	newLocalRules := func() *localRules {
+		return &localRules{
+			LocalCIDR: new(bart.Lite),
+		}
+	}
+
+	if r.isAny() {
+		if rr.Any == nil {
+			rr.Any = newLocalRules()
+		}
+
+		rr.Any.add(r)
+		return
+	}
+
+	if len(r.groups) > 0 {
+		lr := newLocalRules()
+		lr.add(r)
+
+		rr.Groups = append(rr.Groups, &groupsRule{
+			Groups:    r.groups,
+			LocalCIDR: lr,
+		})
+	}
+
+	if r.host != "" {
+		lr := rr.Hosts[r.host]
+		if lr == nil {
+			lr = newLocalRules()
+		}
+		lr.add(r)
+		rr.Hosts[r.host] = lr
+	}
+
+	if r.cidr.IsValid() {
+		lr, _ := rr.CIDR.Get(r.cidr)
+		if lr == nil {
+			lr = newLocalRules()
+		}
+		lr.add(r)
+		rr.CIDR.Insert(r.cidr, lr)
+	}
+}
+
+func (lr *localRules) add(r rule) {
+	if r.anyLocalCIDR {
+		lr.Any = true
+		return
+	}
+
+	for _, prefix := range r.localCIDRs {
+		lr.LocalCIDR.Insert(prefix)
+	}
+}

firewall/table.go

@@ -0,0 +1,164 @@
+package firewall
+
+import (
+	"net/netip"
+
+	"github.com/gaissmai/bart"
+	"github.com/slackhq/nebula/cert"
+	"github.com/slackhq/nebula/iputil"
+)
+
+// Table holds the rules for one direction, 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)
+// A Table doesn't change once built, see RulesBuilder.
+type Table struct {
+	// Protos holds the rules for each IP protocol number, so a packet is only checked against one set of rules.
+	// A protocol with rules of its own also holds a copy of every proto `any` rule, the rest share the proto `any`
+	// rules, which are at Protos[ProtoAny]. nil when a protocol has no rules at all.
+	// ICMP and ICMPv6 share one set of rules.
+	Protos [256]portRules
+}
+
+// Even though ports are uint16, int32 maps are faster for lookup
+// Plus we can use `-1` for fragment rules
+type portRules map[int32]*caRules
+
+type caRules struct {
+	Any     *remoteRules
+	CANames map[string]*remoteRules
+	CAShas  map[string]*remoteRules
+}
+
+type remoteRules struct {
+	// Any makes Hosts, Groups, and CIDR irrelevant
+	Any    *localRules
+	Hosts  map[string]*localRules
+	Groups []*groupsRule
+	CIDR   *bart.Table[*localRules]
+}
+
+type groupsRule struct {
+	Groups    []string
+	LocalCIDR *localRules
+}
+
+type localRules struct {
+	Any       bool
+	LocalCIDR *bart.Lite
+}
+
+// Match reports whether a rule allows p
+func (t *Table) Match(p *Packet, incoming bool, c *cert.CachedCertificate, caPool *cert.CAPool) bool {
+	return t.Protos[p.Protocol].match(p, incoming, c, caPool)
+}
+
+func (pr portRules) match(p *Packet, incoming bool, c *cert.CachedCertificate, caPool *cert.CAPool) bool {
+	// We don't have any allowed ports, bail
+	if pr == nil {
+		return false
+	}
+
+	// ICMP has no ports, only the port `any` rules apply, including the ones copied in from proto `any` 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.
+		return pr[PortAny].match(p, c, caPool)
+	}
+
+	var port int32
+
+	if p.Fragment {
+		port = PortFragment
+	} else if incoming {
+		port = int32(p.LocalPort)
+	} else {
+		port = int32(p.RemotePort)
+	}
+
+	// Packets without ports (gre, esp, etc) have port 0, which is PortAny, don't check those rules twice
+	if port != PortAny && pr[port].match(p, c, caPool) {
+		return true
+	}
+
+	return pr[PortAny].match(p, c, caPool)
+}
+
+func (cr *caRules) match(p *Packet, c *cert.CachedCertificate, caPool *cert.CAPool) bool {
+	if cr == nil {
+		return false
+	}
+
+	if cr.Any.match(p, c) {
+		return true
+	}
+
+	if t, ok := cr.CAShas[c.Certificate.Issuer()]; ok {
+		if t.match(p, c) {
+			return true
+		}
+	}
+
+	s, err := caPool.GetCAForCert(c.Certificate)
+	if err != nil {
+		return false
+	}
+
+	return cr.CANames[s.Certificate.Name()].match(p, c)
+}
+
+func (rr *remoteRules) match(p *Packet, c *cert.CachedCertificate) bool {
+	if rr == nil {
+		return false
+	}
+
+	// Shortcut path for if groups, hosts, or cidr contained an `any`
+	if rr.Any.match(p, c) {
+		return true
+	}
+
+	// Need any of group, host, or cidr to match
+	for _, sg := range rr.Groups {
+		found := false
+
+		for _, g := range sg.Groups {
+			if _, ok := c.InvertedGroups[g]; !ok {
+				found = false
+				break
+			}
+
+			found = true
+		}
+
+		if found && sg.LocalCIDR.match(p, c) {
+			return true
+		}
+	}
+
+	if rr.Hosts != nil {
+		if lr, ok := rr.Hosts[c.Certificate.Name()]; ok {
+			if lr.match(p, c) {
+				return true
+			}
+		}
+	}
+
+	for _, v := range rr.CIDR.Supernets(netip.PrefixFrom(p.RemoteAddr, p.RemoteAddr.BitLen())) {
+		if v.match(p, c) {
+			return true
+		}
+	}
+
+	return false
+}
+
+func (lr *localRules) match(p *Packet, c *cert.CachedCertificate) bool {
+	if lr == nil {
+		return false
+	}
+
+	if lr.Any {
+		return true
+	}
+
+	return lr.LocalCIDR.Contains(p.LocalAddr)
+}