checkpt

firewall/config_test.go

@@ -299,8 +299,17 @@ func TestRulesFromConfig_defaultLocalCIDR(t *testing.T) {
 	vpnAddr := netip.MustParseAddr("10.1.9.9")
 	unsafeAddr := netip.MustParseAddr("192.168.0.3")
 
-	allows := func(lr *localRules, addr netip.Addr) bool {
-		return lr.match(&Packet{LocalAddr: addr}, nil)
+	// ruleOnPort returns the rule on proto whose port clause is exactly port, chosen from the rules that a packet
+	// on port is checked against.
+	ruleOnPort := func(t *testing.T, table *Table, proto uint8, port int32) *rule {
+		t.Helper()
+		for _, r := range table.rulesAt(proto, port) {
+			if r.startPort == port {
+				return r
+			}
+		}
+		require.Failf(t, "no rule found", "proto %d port %d", proto, port)
+		return nil
 	}
 
 	for _, tc := range []struct {
@@ -327,22 +336,24 @@ func TestRulesFromConfig_defaultLocalCIDR(t *testing.T) {
 			rules, err := RulesFromConfig(test.NewLogger(), conf, vpnNetworks, tc.unsafeNetworks)
 			require.NoError(t, err)
 
-			// The proto `any` rule, copied into a protocol with rules of its own and shared by one without
+			// The proto `any` rule, as indexed by a protocol with rules of its own and as shared by one without.
 			for _, proto := range []uint8{iputil.IPProtocolTCP, 47} {
-				lr := rules.In.protos[proto][PortAny].Any.Any
-				assert.True(t, allows(lr, vpnAddr), "proto %d", proto)
-				assert.Equal(t, tc.wantUnsafe, allows(lr, unsafeAddr), "proto %d", proto)
+				r := ruleOnPort(t, rules.In, proto, PortAny)
+				assert.True(t, r.matchLocal(vpnAddr), "proto %d", proto)
+				assert.Equal(t, tc.wantUnsafe, r.matchLocal(unsafeAddr), "proto %d", proto)
 			}
 
-			tcp := rules.In.protos[iputil.IPProtocolTCP]
-			assert.True(t, allows(tcp[22].Any.Any, vpnAddr))
-			assert.Equal(t, tc.wantUnsafe, allows(tcp[22].Any.Any, unsafeAddr))
+			r := ruleOnPort(t, rules.In, iputil.IPProtocolTCP, 22)
+			assert.True(t, r.matchLocal(vpnAddr))
+			assert.Equal(t, tc.wantUnsafe, r.matchLocal(unsafeAddr))
 
-			assert.False(t, allows(tcp[80].Any.Any, vpnAddr))
-			assert.True(t, allows(tcp[80].Any.Any, unsafeAddr))
+			r = ruleOnPort(t, rules.In, iputil.IPProtocolTCP, 80)
+			assert.False(t, r.matchLocal(vpnAddr))
+			assert.True(t, r.matchLocal(unsafeAddr))
 
-			assert.True(t, allows(tcp[443].Any.Any, vpnAddr))
-			assert.True(t, allows(tcp[443].Any.Any, unsafeAddr))
+			r = ruleOnPort(t, rules.In, iputil.IPProtocolTCP, 443)
+			assert.True(t, r.matchLocal(vpnAddr))
+			assert.True(t, r.matchLocal(unsafeAddr))
 		})
 	}
 }

firewall/rules_builder.go

@@ -8,10 +8,10 @@ import (
 	"hash/fnv"
 	"io"
 	"log/slog"
+	"math"
 	"net/netip"
 	"slices"
 
-	"github.com/gaissmai/bart"
 	"github.com/slackhq/nebula/iputil"
 )
 
@@ -36,24 +36,6 @@ type tableRules struct {
 	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
-
-	// localCIDR is 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.
-	// localCIDR is shared by every localRules the rule is added to, so it's never changed, see localRules.add
-	localCIDR    *bart.Lite
-	anyLocalCIDR bool
-}
-
 func NewRulesBuilder(l *slog.Logger) *RulesBuilder {
 	return &RulesBuilder{
 		ruleSHA: sha256.New(),
@@ -105,12 +87,10 @@ func (b *RulesBuilder) AddRule(incoming bool, proto uint8, startPort int32, endP
 // 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 {
-	var defaultLocal *bart.Lite
+	var defaultLocal []netip.Prefix
 	if len(unsafeNetworks) > 0 && !b.defaultLocalCIDRAny {
-		defaultLocal = new(bart.Lite)
-		for _, network := range vpnNetworks {
-			defaultLocal.Insert(network)
-		}
+		// Never nil, even when there are no vpn networks, so that the rules allow nothing rather than everything.
+		defaultLocal = append([]netip.Prefix{}, vpnNetworks...)
 	}
 
 	return Rules{
@@ -125,6 +105,13 @@ func parseRule(startPort, endPort int32, groups []string, host, cidr, localCidr,
 	if startPort > endPort {
 		return rule{}, fmt.Errorf("start port was lower than end port")
 	}
+	if startPort < PortFragment || endPort > math.MaxUint16 {
+		return rule{}, fmt.Errorf("port range %d-%d is outside of %d-%d", startPort, endPort, PortFragment, math.MaxUint16)
+	}
+	// A range that includes port 0 is port `any`.
+	if startPort <= PortAny && PortAny <= endPort {
+		startPort, endPort = PortAny, PortAny
+	}
 
 	r := rule{
 		startPort: startPort,
@@ -135,20 +122,24 @@ func parseRule(startPort, endPort int32, groups []string, host, cidr, localCidr,
 		caSha:     caSha,
 	}
 
+	var anyCIDR bool
 	var err error
-	r.cidr, r.anyCIDR, err = parseCIDR(cidr)
+	r.cidr, anyCIDR, err = parseCIDR(cidr)
 	if err != nil {
 		return rule{}, fmt.Errorf("cidr did not parse; %w", err)
 	}
 
+	// A rule with no group, host, or cidr allows any host, as does a rule with `any` for one of them.
+	r.remoteAny = anyCIDR || host == "any" || slices.Contains(groups, "any") ||
+		(len(groups) == 0 && host == "" && !r.cidr.IsValid())
+
 	var localCIDR netip.Prefix
-	localCIDR, r.anyLocalCIDR, err = parseCIDR(localCidr)
+	localCIDR, r.localAny, err = parseCIDR(localCidr)
 	if err != nil {
 		return rule{}, fmt.Errorf("local_cidr did not parse; %w", err)
 	}
 	if localCIDR.IsValid() {
-		r.localCIDR = new(bart.Lite)
-		r.localCIDR.Insert(localCIDR)
+		r.local = []netip.Prefix{localCIDR}
 	}
 
 	return r, nil
@@ -168,40 +159,96 @@ func (tr *tableRules) add(proto uint8, r rule) {
 
 // build turns the rules into a Table. Rules without a local cidr allow defaultLocal,
 // or any local address when defaultLocal is nil.
-func (tr *tableRules) build(defaultLocal *bart.Lite) *Table {
+func (tr *tableRules) build(defaultLocal []netip.Prefix) *Table {
 	t := &Table{}
 
-	// Every protocol starts out sharing the proto `any` rules, including protos[ProtoAny]
-	anyProto := newPortRules(defaultLocal, tr.anyRules)
-	for proto := range t.protos {
-		t.protos[proto] = anyProto
+	// A rule's id is its index in t.rules.
+	add := func(rules []rule) []int {
+		ids := make([]int, 0, len(rules))
+		for _, r := range rules {
+			ids = append(ids, len(t.rules))
+			t.rules = append(t.rules, r.withDefaultLocal(defaultLocal))
+		}
+		return ids
+	}
+	anyIDs := add(tr.anyRules)
+	var protoIDs [256][]int
+	for proto, rules := range tr.protoRules {
+		protoIDs[proto] = add(rules)
 	}
 
-	// Protocols with rules of their own get a copy of the proto `any` rules too, so a packet only checks one set
-	for proto, rules := range tr.protoRules {
-		if len(rules) > 0 {
-			t.protos[proto] = newPortRules(defaultLocal, tr.anyRules, rules)
+	// Every protocol starts out sharing an index of the proto `any` rules, including protos[ProtoAny]. There is
+	// one for protocols with ports and one for protocols without.
+	anyPorts := newProtoIndex(true, t.rules, anyIDs)
+	anyNoPorts := newProtoIndex(false, t.rules, anyIDs)
+	for proto := range t.protos {
+		if hasPorts(uint8(proto)) {
+			t.protos[proto] = anyPorts
+		} else {
+			t.protos[proto] = anyNoPorts
 		}
 	}
 
-	// ICMP and ICMPv6 share one set of rules, add keeps them together under ICMP
+	// A protocol with rules of its own gets an index of those rules plus the proto `any` rules, so a packet
+	// checks a single index.
+	for proto, ids := range protoIDs {
+		if len(ids) > 0 {
+			t.protos[proto] = newProtoIndex(hasPorts(uint8(proto)), t.rules, anyIDs, ids)
+		}
+	}
+
+	// ICMP and ICMPv6 share one set of rules; add keeps them together under ICMP.
 	t.protos[iputil.IPProtocolICMPv6] = t.protos[iputil.IPProtocolICMP]
 
 	return t
 }
 
-// newPortRules builds portRules from each set of rules, or returns nil when there are none
-func newPortRules(defaultLocal *bart.Lite, ruleSets ...[]rule) portRules {
-	var pr portRules
-	for _, rules := range ruleSets {
-		for _, r := range rules {
-			if pr == nil {
-				pr = portRules{}
-			}
-			pr.add(r.withDefaultLocal(defaultLocal))
+// newProtoIndex builds an index of the rules with the given ids for a protocol with or without ports, or
+// returns nil when there are none.
+func newProtoIndex(hasPorts bool, rules []rule, idSets ...[]int) *protoIndex {
+	ids := slices.Concat(idSets...)
+	if len(ids) == 0 {
+		return nil
+	}
+
+	pi := &protoIndex{hasPorts: hasPorts, fragment: newRuleSet(len(rules))}
+	if hasPorts {
+		pi.byPort = newRuleSets(math.MaxUint16+1, len(rules))
+	} else {
+		pi.packet = newRuleSet(len(rules))
+	}
+
+	for _, id := range ids {
+		pi.add(&rules[id], id)
+	}
+	return pi
+}
+
+// add puts id in every set that r's port clause covers.
+func (pi *protoIndex) add(r *rule, id int) {
+	switch r.startPort {
+	case PortAny:
+		pi.fragment.add(id)
+		if pi.hasPorts {
+			pi.addPorts(0, math.MaxUint16, id)
+		} else {
+			pi.packet.add(id)
+		}
+	case PortFragment:
+		pi.fragment.add(id)
+	default:
+		// A range only applies to a protocol with ports.
+		if pi.hasPorts {
+			pi.addPorts(int(r.startPort), int(r.endPort), id)
 		}
 	}
-	return pr
+}
+
+// addPorts puts id in the set of every port from lo through hi.
+func (pi *protoIndex) addPorts(lo, hi, id int) {
+	for port := lo; port <= hi; port++ {
+		pi.byPort.at(port).add(id)
+	}
 }
 
 // parseCIDR parses a rule's cidr, which is empty, `any`, or a prefix
@@ -217,112 +264,12 @@ func parseCIDR(s string) (prefix netip.Prefix, isAny bool, err error) {
 	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
-}
-
 // withDefaultLocal returns r allowing defaultLocal, or any local address when defaultLocal is nil,
 // unless r has a local cidr of its own
-func (r rule) withDefaultLocal(defaultLocal *bart.Lite) rule {
-	if !r.anyLocalCIDR && r.localCIDR == nil {
-		r.anyLocalCIDR = defaultLocal == nil
-		r.localCIDR = defaultLocal
+func (r rule) withDefaultLocal(defaultLocal []netip.Prefix) rule {
+	if !r.localAny && r.local == nil {
+		r.localAny = defaultLocal == nil
+		r.local = defaultLocal
 	}
 	return r
 }
-
-func (pr portRules) add(r rule) {
-	for i := r.startPort; i <= r.endPort; i++ {
-		cr := pr[i]
-		if cr == nil {
-			cr = &caRules{}
-			pr[i] = cr
-		}
-		cr.add(r)
-	}
-}
-
-func (cr *caRules) add(r rule) {
-	if r.caSha == "" && r.caName == "" {
-		if cr.Any == nil {
-			cr.Any = &remoteRules{}
-		}
-		cr.Any.add(r)
-		return
-	}
-
-	if r.caSha != "" {
-		getOrNew(&cr.CAShas, r.caSha).add(r)
-	}
-
-	if r.caName != "" {
-		getOrNew(&cr.CANames, r.caName).add(r)
-	}
-}
-
-func (rr *remoteRules) add(r rule) {
-	if r.isAny() {
-		if rr.Any == nil {
-			rr.Any = &localRules{}
-		}
-		rr.Any.add(r)
-		return
-	}
-
-	if len(r.groups) > 0 {
-		lr := &localRules{}
-		lr.add(r)
-		rr.Groups = append(rr.Groups, &groupsRule{
-			Groups:    r.groups,
-			LocalCIDR: lr,
-		})
-	}
-
-	if r.host != "" {
-		getOrNew(&rr.Hosts, r.host).add(r)
-	}
-
-	if r.cidr.IsValid() {
-		prefix := r.cidr.Masked()
-		i := slices.IndexFunc(rr.CIDR, func(cr cidrRule) bool { return cr.Prefix == prefix })
-		if i < 0 {
-			i = len(rr.CIDR)
-			rr.CIDR = append(rr.CIDR, cidrRule{Prefix: prefix, LocalCIDR: &localRules{}})
-		}
-		rr.CIDR[i].LocalCIDR.add(r)
-	}
-}
-
-func (lr *localRules) add(r rule) {
-	if r.anyLocalCIDR {
-		lr.Any = true
-		return
-	}
-
-	// Share the rule's local cidrs until another rule's need adding, then copy them instead of changing them
-	if lr.LocalCIDR == nil {
-		lr.LocalCIDR = r.localCIDR
-		return
-	}
-	for prefix := range r.localCIDR.All() {
-		lr.LocalCIDR = lr.LocalCIDR.InsertPersist(prefix)
-	}
-}
-
-// getOrNew returns m[k], adding a new value for k first when there isn't one. m is made if it's nil.
-func getOrNew[M ~map[K]*V, K comparable, V any](m *M, k K) *V {
-	if *m == nil {
-		*m = make(M)
-	}
-	v := (*m)[k]
-	if v == nil {
-		v = new(V)
-		(*m)[k] = v
-	}
-	return v
-}

firewall/rules_builder_test.go

@@ -1,166 +1,198 @@
 package firewall
 
 import (
+	"fmt"
 	"math"
 	"net/netip"
 	"testing"
+	"time"
 
+	"github.com/slackhq/nebula/cert"
+	"github.com/slackhq/nebula/cert_test"
 	"github.com/slackhq/nebula/iputil"
 	"github.com/slackhq/nebula/test"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
+// rulesAt returns the rules that a packet on proto with the given port is checked against, in id order.
+// port may also be PortAny or PortFragment.
+func (t *Table) rulesAt(proto uint8, port int32) []*rule {
+	pi := t.protos[proto]
+	if pi == nil {
+		return nil
+	}
+
+	p := &Packet{Protocol: proto, Fragment: port == PortFragment}
+	if port > 0 {
+		p.LocalPort = uint16(port)
+	}
+
+	var rules []*rule
+	for id := range pi.whichMatch(p, true).all() {
+		rules = append(rules, &t.rules[id])
+	}
+	return rules
+}
+
+// oneRuleAt returns the single rule that rulesAt finds, and fails the test when there is not exactly one.
+func oneRuleAt(t *testing.T, table *Table, proto uint8, key int32) *rule {
+	t.Helper()
+	rules := table.rulesAt(proto, key)
+	require.Len(t, rules, 1)
+	return rules[0]
+}
+
 func TestRulesBuilder_AddRule(t *testing.T) {
 	l := test.NewLogger()
 
-	ti, err := netip.ParsePrefix("1.2.3.4/32")
-	require.NoError(t, err)
-
-	ti6, err := netip.ParsePrefix("fd12::34/128")
-	require.NoError(t, err)
+	ti := netip.MustParsePrefix("1.2.3.4/32")
+	ti6 := netip.MustParsePrefix("fd12::34/128")
 
 	rb := NewRulesBuilder(l)
 	require.NoError(t, rb.AddRule(true, iputil.IPProtocolTCP, 1, 1, []string{}, "", "", "", "", ""))
 	rules := rb.Build(nil, nil)
 	// An empty rule is any
-	assert.True(t, rules.In.protos[iputil.IPProtocolTCP][1].Any.Any.Any)
-	assert.Empty(t, rules.In.protos[iputil.IPProtocolTCP][1].Any.Groups)
-	assert.Empty(t, rules.In.protos[iputil.IPProtocolTCP][1].Any.Hosts)
+	r := oneRuleAt(t, rules.In, iputil.IPProtocolTCP, 1)
+	assert.True(t, r.remoteAny)
+	assert.Empty(t, r.groups)
+	assert.Empty(t, r.host)
+	// It applies only to its own port.
+	assert.Empty(t, rules.In.rulesAt(iputil.IPProtocolTCP, 2))
+	assert.Empty(t, rules.In.rulesAt(iputil.IPProtocolTCP, PortAny))
 
 	rb = NewRulesBuilder(l)
 	require.NoError(t, rb.AddRule(true, iputil.IPProtocolUDP, 1, 1, []string{"g1"}, "", "", "", "", ""))
 	rules = rb.Build(nil, nil)
-	assert.Nil(t, rules.In.protos[iputil.IPProtocolUDP][1].Any.Any)
-	assert.Contains(t, rules.In.protos[iputil.IPProtocolUDP][1].Any.Groups[0].Groups, "g1")
-	assert.Empty(t, rules.In.protos[iputil.IPProtocolUDP][1].Any.Hosts)
+	r = oneRuleAt(t, rules.In, iputil.IPProtocolUDP, 1)
+	assert.False(t, r.remoteAny)
+	assert.Equal(t, []string{"g1"}, r.groups)
+	assert.Empty(t, r.host)
 
 	rb = NewRulesBuilder(l)
 	require.NoError(t, rb.AddRule(true, iputil.IPProtocolICMP, 1, 1, []string{}, "h1", "", "", "", ""))
 	require.NoError(t, rb.AddRule(true, iputil.IPProtocolICMPv6, 1, 1, []string{}, "h2", "", "", "", ""))
 	rules = rb.Build(nil, nil)
-	//no matter what port is given for icmp, it should end up as "any"
-	assert.Nil(t, rules.In.protos[iputil.IPProtocolICMP][PortAny].Any.Any)
-	assert.Empty(t, rules.In.protos[iputil.IPProtocolICMP][PortAny].Any.Groups)
-	assert.Contains(t, rules.In.protos[iputil.IPProtocolICMP][PortAny].Any.Hosts, "h1")
-
-	// ICMP and ICMPv6 share one set of rules
-	assert.Contains(t, rules.In.protos[iputil.IPProtocolICMP][PortAny].Any.Hosts, "h2")
-	assert.Contains(t, rules.In.protos[iputil.IPProtocolICMPv6][PortAny].Any.Hosts, "h1")
+	// An ICMP rule is port `any` regardless of the port given, and ICMP and ICMPv6 share one set of rules.
+	for _, proto := range []uint8{iputil.IPProtocolICMP, iputil.IPProtocolICMPv6} {
+		rs := rules.In.rulesAt(proto, PortAny)
+		require.Len(t, rs, 2, "proto %d", proto)
+		assert.False(t, rs[0].remoteAny)
+		assert.Equal(t, "h1", rs[0].host)
+		assert.Equal(t, "h2", rs[1].host)
+	}
+	assert.Same(t, rules.In.protos[iputil.IPProtocolICMP], rules.In.protos[iputil.IPProtocolICMPv6])
 
 	// Any other protocol number gets its own rules
 	rb = NewRulesBuilder(l)
 	require.NoError(t, rb.AddRule(true, 47, 0, 0, []string{"g1"}, "", "", "", "", "")) // GRE
 	rules = rb.Build(nil, nil)
-	assert.Contains(t, rules.In.protos[47][PortAny].Any.Groups[0].Groups, "g1")
+	assert.Equal(t, []string{"g1"}, oneRuleAt(t, rules.In, 47, PortAny).groups)
 	assert.Nil(t, rules.In.protos[iputil.IPProtocolTCP])
 
+	// A proto `any` rule with a port applies to every protocol with ports, and ProtoAny itself has none.
 	rb = NewRulesBuilder(l)
 	require.NoError(t, rb.AddRule(false, ProtoAny, 1, 1, []string{}, "", ti.String(), "", "", ""))
 	rules = rb.Build(nil, nil)
-	assert.Nil(t, rules.Out.protos[ProtoAny][1].Any.Any)
-	require.Len(t, rules.Out.protos[ProtoAny][1].Any.CIDR, 1)
-	assert.Equal(t, ti, rules.Out.protos[ProtoAny][1].Any.CIDR[0].Prefix)
+	assert.Empty(t, rules.Out.rulesAt(ProtoAny, 1))
+	r = oneRuleAt(t, rules.Out, iputil.IPProtocolTCP, 1)
+	assert.False(t, r.remoteAny)
+	assert.Equal(t, ti, r.cidr)
 
 	rb = NewRulesBuilder(l)
 	require.NoError(t, rb.AddRule(false, ProtoAny, 1, 1, []string{}, "", ti6.String(), "", "", ""))
 	rules = rb.Build(nil, nil)
-	assert.Nil(t, rules.Out.protos[ProtoAny][1].Any.Any)
-	require.Len(t, rules.Out.protos[ProtoAny][1].Any.CIDR, 1)
-	assert.Equal(t, ti6, rules.Out.protos[ProtoAny][1].Any.CIDR[0].Prefix)
-
-	// The same cidr written two ways is one entry
-	rb = NewRulesBuilder(l)
-	require.NoError(t, rb.AddRule(false, ProtoAny, 1, 1, []string{}, "", "1.2.3.4/24", "", "", ""))
-	require.NoError(t, rb.AddRule(false, ProtoAny, 1, 1, []string{}, "", "1.2.3.0/24", "", "", ""))
-	rules = rb.Build(nil, nil)
-	require.Len(t, rules.Out.protos[ProtoAny][1].Any.CIDR, 1)
-	assert.Equal(t, netip.MustParsePrefix("1.2.3.0/24"), rules.Out.protos[ProtoAny][1].Any.CIDR[0].Prefix)
+	r = oneRuleAt(t, rules.Out, iputil.IPProtocolTCP, 1)
+	assert.False(t, r.remoteAny)
+	assert.Equal(t, ti6, r.cidr)
 
 	rb = NewRulesBuilder(l)
 	require.NoError(t, rb.AddRule(false, ProtoAny, 1, 1, []string{}, "", "", ti.String(), "", ""))
 	rules = rb.Build(nil, nil)
-	assert.NotNil(t, rules.Out.protos[ProtoAny][1].Any.Any)
-	ok := rules.Out.protos[ProtoAny][1].Any.Any.LocalCIDR.Get(ti)
-	assert.True(t, ok)
+	r = oneRuleAt(t, rules.Out, iputil.IPProtocolTCP, 1)
+	assert.True(t, r.remoteAny)
+	assert.False(t, r.localAny)
+	assert.Equal(t, []netip.Prefix{ti}, r.local)
 
 	rb = NewRulesBuilder(l)
 	require.NoError(t, rb.AddRule(false, ProtoAny, 1, 1, []string{}, "", "", ti6.String(), "", ""))
 	rules = rb.Build(nil, nil)
-	assert.NotNil(t, rules.Out.protos[ProtoAny][1].Any.Any)
-	ok = rules.Out.protos[ProtoAny][1].Any.Any.LocalCIDR.Get(ti6)
-	assert.True(t, ok)
+	r = oneRuleAt(t, rules.Out, iputil.IPProtocolTCP, 1)
+	assert.True(t, r.remoteAny)
+	assert.False(t, r.localAny)
+	assert.Equal(t, []netip.Prefix{ti6}, r.local)
 
 	rb = NewRulesBuilder(l)
 	require.NoError(t, rb.AddRule(true, iputil.IPProtocolUDP, 1, 1, []string{"g1"}, "", "", "", "ca-name", ""))
 	rules = rb.Build(nil, nil)
-	assert.Contains(t, rules.In.protos[iputil.IPProtocolUDP][1].CANames, "ca-name")
+	assert.Equal(t, "ca-name", oneRuleAt(t, rules.In, iputil.IPProtocolUDP, 1).caName)
 
 	rb = NewRulesBuilder(l)
 	require.NoError(t, rb.AddRule(true, iputil.IPProtocolUDP, 1, 1, []string{"g1"}, "", "", "", "", "ca-sha"))
 	rules = rb.Build(nil, nil)
-	assert.Contains(t, rules.In.protos[iputil.IPProtocolUDP][1].CAShas, "ca-sha")
+	assert.Equal(t, "ca-sha", oneRuleAt(t, rules.In, iputil.IPProtocolUDP, 1).caSha)
 
 	rb = NewRulesBuilder(l)
 	require.NoError(t, rb.AddRule(false, ProtoAny, 0, 0, []string{}, "any", "", "", "", ""))
 	rules = rb.Build(nil, nil)
-	assert.True(t, rules.Out.protos[ProtoAny][0].Any.Any.Any)
+	assert.True(t, oneRuleAt(t, rules.Out, ProtoAny, 0).remoteAny)
 
+	// 0.0.0.0/0 allows any IPv4 address, but no IPv6 address.
 	rb = NewRulesBuilder(l)
-	anyIp, err := netip.ParsePrefix("0.0.0.0/0")
-	require.NoError(t, err)
-
-	// 0.0.0.0/0 is any IPv4 address, not any IPv6 address
-	require.NoError(t, rb.AddRule(false, ProtoAny, 0, 0, []string{}, "", anyIp.String(), "", "", ""))
+	require.NoError(t, rb.AddRule(false, ProtoAny, 0, 0, []string{}, "", "0.0.0.0/0", "", "", ""))
 	rules = rb.Build(nil, nil)
-	assert.Nil(t, rules.Out.protos[ProtoAny][0].Any.Any)
-	assert.True(t, rules.Out.protos[ProtoAny][0].Any.match(&Packet{RemoteAddr: netip.MustParseAddr("1.1.1.1")}, nil))
-	assert.False(t, rules.Out.protos[ProtoAny][0].Any.match(&Packet{RemoteAddr: netip.MustParseAddr("9::9")}, nil))
+	r = oneRuleAt(t, rules.Out, ProtoAny, 0)
+	assert.False(t, r.remoteAny)
+	assert.True(t, r.matchRemote(netip.MustParseAddr("1.1.1.1"), nil))
+	assert.False(t, r.matchRemote(netip.MustParseAddr("9::9"), nil))
 
+	// ::/0 allows any IPv6 address, but no IPv4 address.
 	rb = NewRulesBuilder(l)
-	anyIp6, err := netip.ParsePrefix("::/0")
-	require.NoError(t, err)
-
-	// ::/0 is any IPv6 address, not any IPv4 address
-	require.NoError(t, rb.AddRule(false, ProtoAny, 0, 0, []string{}, "", anyIp6.String(), "", "", ""))
+	require.NoError(t, rb.AddRule(false, ProtoAny, 0, 0, []string{}, "", "::/0", "", "", ""))
 	rules = rb.Build(nil, nil)
-	assert.Nil(t, rules.Out.protos[ProtoAny][0].Any.Any)
-	assert.True(t, rules.Out.protos[ProtoAny][0].Any.match(&Packet{RemoteAddr: netip.MustParseAddr("9::9")}, nil))
-	assert.False(t, rules.Out.protos[ProtoAny][0].Any.match(&Packet{RemoteAddr: netip.MustParseAddr("1.1.1.1")}, nil))
+	r = oneRuleAt(t, rules.Out, ProtoAny, 0)
+	assert.False(t, r.remoteAny)
+	assert.True(t, r.matchRemote(netip.MustParseAddr("9::9"), nil))
+	assert.False(t, r.matchRemote(netip.MustParseAddr("1.1.1.1"), nil))
 
 	rb = NewRulesBuilder(l)
 	require.NoError(t, rb.AddRule(false, ProtoAny, 0, 0, []string{}, "", "any", "", "", ""))
 	rules = rb.Build(nil, nil)
-	assert.True(t, rules.Out.protos[ProtoAny][0].Any.Any.Any)
+	assert.True(t, oneRuleAt(t, rules.Out, ProtoAny, 0).remoteAny)
+
+	// A local cidr is likewise limited to its own address family.
+	rb = NewRulesBuilder(l)
+	require.NoError(t, rb.AddRule(false, ProtoAny, 0, 0, []string{}, "", "", "0.0.0.0/0", "", ""))
+	rules = rb.Build(nil, nil)
+	r = oneRuleAt(t, rules.Out, ProtoAny, 0)
+	assert.False(t, r.localAny)
+	assert.True(t, r.matchLocal(netip.MustParseAddr("1.1.1.1")))
+	assert.False(t, r.matchLocal(netip.MustParseAddr("9::9")))
 
 	rb = NewRulesBuilder(l)
-	require.NoError(t, rb.AddRule(false, ProtoAny, 0, 0, []string{}, "", "", anyIp.String(), "", ""))
+	require.NoError(t, rb.AddRule(false, ProtoAny, 0, 0, []string{}, "", "", "::/0", "", ""))
 	rules = rb.Build(nil, nil)
-	assert.False(t, rules.Out.protos[ProtoAny][0].Any.Any.Any)
-	assert.True(t, rules.Out.protos[ProtoAny][0].Any.Any.LocalCIDR.Lookup(netip.MustParseAddr("1.1.1.1")))
-	assert.False(t, rules.Out.protos[ProtoAny][0].Any.Any.LocalCIDR.Lookup(netip.MustParseAddr("9::9")))
-
-	rb = NewRulesBuilder(l)
-	require.NoError(t, rb.AddRule(false, ProtoAny, 0, 0, []string{}, "", "", anyIp6.String(), "", ""))
-	rules = rb.Build(nil, nil)
-	assert.False(t, rules.Out.protos[ProtoAny][0].Any.Any.Any)
-	assert.True(t, rules.Out.protos[ProtoAny][0].Any.Any.LocalCIDR.Lookup(netip.MustParseAddr("9::9")))
-	assert.False(t, rules.Out.protos[ProtoAny][0].Any.Any.LocalCIDR.Lookup(netip.MustParseAddr("1.1.1.1")))
+	r = oneRuleAt(t, rules.Out, ProtoAny, 0)
+	assert.False(t, r.localAny)
+	assert.True(t, r.matchLocal(netip.MustParseAddr("9::9")))
+	assert.False(t, r.matchLocal(netip.MustParseAddr("1.1.1.1")))
 
 	rb = NewRulesBuilder(l)
 	require.NoError(t, rb.AddRule(false, ProtoAny, 0, 0, []string{}, "", "", "any", "", ""))
 	rules = rb.Build(nil, nil)
-	assert.True(t, rules.Out.protos[ProtoAny][0].Any.Any.Any)
+	assert.True(t, oneRuleAt(t, rules.Out, ProtoAny, 0).localAny)
 
 	// Every protocol number is accepted
 	rb = NewRulesBuilder(l)
 	require.NoError(t, rb.AddRule(true, math.MaxUint8, 0, 0, []string{}, "", "", "", "", ""))
 	rules = rb.Build(nil, nil)
-	assert.True(t, rules.In.protos[math.MaxUint8][PortAny].Any.Any.Any)
+	assert.True(t, oneRuleAt(t, rules.In, math.MaxUint8, PortAny).remoteAny)
 
 	// Test error conditions, a bad rule is reported when it's added
 	rb = NewRulesBuilder(l)
 	require.Error(t, rb.AddRule(true, ProtoAny, 10, 0, []string{}, "", "", "", "", ""))
+	require.Error(t, rb.AddRule(true, ProtoAny, PortFragment-1, 0, []string{}, "", "", "", "", ""))
+	require.Error(t, rb.AddRule(true, ProtoAny, 1, math.MaxUint16+1, []string{}, "", "", "", "", ""))
 	require.Error(t, rb.AddRule(true, ProtoAny, 0, 0, []string{}, "", "junk", "", "", ""))
 	require.Error(t, rb.AddRule(true, ProtoAny, 0, 0, []string{}, "", "", "junk", "", ""))
 
@@ -169,8 +201,9 @@ func TestRulesBuilder_AddRule(t *testing.T) {
 	require.NoError(t, rb.AddRule(true, ProtoAny, PortAny, PortAny, []string{"default-group"}, "", "", "", "", ""))
 	require.NoError(t, rb.AddRule(true, iputil.IPProtocolICMPv6, PortAny, PortAny, []string{"nope"}, "", "", "", "", ""))
 	rules = rb.Build(nil, nil)
-	assert.Len(t, rules.In.protos[iputil.IPProtocolICMP][PortAny].Any.Groups, 2)
-	assert.Len(t, rules.In.protos[iputil.IPProtocolICMPv6][PortAny].Any.Groups, 2)
+	assert.Len(t, rules.In.rules, 2)
+	assert.Len(t, rules.In.rulesAt(iputil.IPProtocolICMP, PortAny), 2)
+	assert.Len(t, rules.In.rulesAt(iputil.IPProtocolICMPv6, PortAny), 2)
 
 	// No rules allow nothing
 	rules = NewRulesBuilder(l).Build(nil, nil)
@@ -180,10 +213,84 @@ func TestRulesBuilder_AddRule(t *testing.T) {
 	}
 }
 
-// TestRulesBuilder_sharedLocalCIDR ensures local cidrs shared between rules and ports are copied, not changed,
-// when another rule's local cidrs are added to them. The prefixes share their first octet so the copy has to reach
-// past the root of the shared table.
-func TestRulesBuilder_sharedLocalCIDR(t *testing.T) {
+// TestRulesBuilder_protoIndex checks which packets each kind of port clause covers, and which protocols
+// share an index.
+func TestRulesBuilder_protoIndex(t *testing.T) {
+	rb := NewRulesBuilder(test.NewLogger())
+	require.NoError(t, rb.AddRule(true, ProtoAny, PortAny, PortAny, nil, "any", "", "", "", ""))
+	require.NoError(t, rb.AddRule(true, iputil.IPProtocolTCP, 22, 22, nil, "port", "", "", "", ""))
+	require.NoError(t, rb.AddRule(true, iputil.IPProtocolTCP, 1024, math.MaxUint16, nil, "range", "", "", "", ""))
+	require.NoError(t, rb.AddRule(true, iputil.IPProtocolTCP, PortFragment, PortFragment, nil, "fragment", "", "", "", ""))
+	// A range that includes port 0 is port `any`.
+	require.NoError(t, rb.AddRule(true, iputil.IPProtocolUDP, 0, 100, nil, "zero", "", "", "", ""))
+	// GRE has no ports, so a port rule on it never applies.
+	require.NoError(t, rb.AddRule(true, 47, 22, 22, nil, "gre", "", "", "", ""))
+	rules := rb.Build(nil, nil)
+
+	hosts := func(proto uint8, key int32) []string {
+		var hosts []string
+		for _, r := range rules.In.rulesAt(proto, key) {
+			hosts = append(hosts, r.host)
+		}
+		return hosts
+	}
+
+	assert.Equal(t, []string{"any", "fragment"}, hosts(iputil.IPProtocolTCP, PortFragment))
+	assert.Equal(t, []string{"any"}, hosts(iputil.IPProtocolTCP, PortAny))
+	assert.Equal(t, []string{"any"}, hosts(iputil.IPProtocolTCP, 21))
+	assert.Equal(t, []string{"any", "port"}, hosts(iputil.IPProtocolTCP, 22))
+	assert.Equal(t, []string{"any"}, hosts(iputil.IPProtocolTCP, 23))
+	assert.Equal(t, []string{"any"}, hosts(iputil.IPProtocolTCP, 1023))
+	assert.Equal(t, []string{"any", "range"}, hosts(iputil.IPProtocolTCP, 1024))
+	assert.Equal(t, []string{"any", "range"}, hosts(iputil.IPProtocolTCP, math.MaxUint16))
+
+	assert.Equal(t, []string{"any", "zero"}, hosts(iputil.IPProtocolUDP, PortFragment))
+	assert.Equal(t, []string{"any", "zero"}, hosts(iputil.IPProtocolUDP, PortAny))
+	assert.Equal(t, []string{"any", "zero"}, hosts(iputil.IPProtocolUDP, math.MaxUint16))
+
+	// A protocol without ports sees only the port `any` rules, and the port `fragment` rules for a fragment.
+	assert.False(t, rules.In.protos[47].hasPorts)
+	assert.Equal(t, []string{"any"}, hosts(47, PortAny))
+	assert.Equal(t, []string{"any"}, hosts(47, 22))
+	assert.Equal(t, []string{"any"}, hosts(47, PortFragment))
+	assert.Equal(t, []string{"any"}, hosts(iputil.IPProtocolICMP, PortFragment))
+
+	// Protocols without rules of their own share an index of the proto `any` rules: one for protocols with
+	// ports and one for protocols without.
+	assert.True(t, rules.In.protos[iputil.IPProtocolSCTP].hasPorts)
+	assert.Equal(t, []string{"any"}, hosts(iputil.IPProtocolSCTP, 5000))
+	assert.Same(t, rules.In.protos[iputil.IPProtocolSCTP], rules.In.protos[iputil.IPProtocolDCCP])
+	assert.NotSame(t, rules.In.protos[iputil.IPProtocolSCTP], rules.In.protos[iputil.IPProtocolTCP])
+	assert.Equal(t, []string{"any"}, hosts(50, PortAny))
+	assert.Same(t, rules.In.protos[ProtoAny], rules.In.protos[50])
+	assert.Same(t, rules.In.protos[ProtoAny], rules.In.protos[iputil.IPProtocolICMP])
+	assert.NotSame(t, rules.In.protos[ProtoAny], rules.In.protos[iputil.IPProtocolSCTP])
+	assert.Nil(t, rules.Out.protos[iputil.IPProtocolTCP])
+}
+
+// TestRulesBuilder_manyRules checks that rule ids beyond the first 64 are indexed correctly.
+func TestRulesBuilder_manyRules(t *testing.T) {
+	rb := NewRulesBuilder(test.NewLogger())
+	const n = 130
+	for i := range n {
+		require.NoError(t, rb.AddRule(true, iputil.IPProtocolTCP, int32(i+1), int32(i+1), nil, fmt.Sprintf("h%d", i), "", "", "", ""))
+	}
+	require.NoError(t, rb.AddRule(true, ProtoAny, PortAny, PortAny, nil, "any", "", "", "", ""))
+	rules := rb.Build(nil, nil)
+
+	assert.Equal(t, 3, rules.In.protos[iputil.IPProtocolTCP].byPort.setLen)
+	for i := range n {
+		rs := rules.In.rulesAt(iputil.IPProtocolTCP, int32(i+1))
+		require.Len(t, rs, 2, "port %d", i+1)
+		assert.Equal(t, "any", rs[0].host)
+		assert.Equal(t, fmt.Sprintf("h%d", i), rs[1].host)
+	}
+	assert.Len(t, rules.In.rulesAt(iputil.IPProtocolTCP, n+1), 1)
+}
+
+// TestRulesBuilder_defaultLocalCIDR checks which local addresses rules allow when there are unsafe networks.
+// A rule without a local cidr allows only the vpn networks, and a rule with one allows only that cidr.
+func TestRulesBuilder_defaultLocalCIDR(t *testing.T) {
 	vpnNetworks := []netip.Prefix{
 		netip.MustParsePrefix("10.1.0.5/16"),
 		netip.MustParsePrefix("10.2.0.5/16"),
@@ -209,8 +316,19 @@ func TestRulesBuilder_sharedLocalCIDR(t *testing.T) {
 	require.NoError(t, rb.AddRule(true, iputil.IPProtocolTCP, 3, 3, nil, "h3", "", "", "", ""))
 	rules := rb.Build(vpnNetworks, unsafeNetworks)
 
+	ca, _, caKey, _ := cert_test.NewTestCaCert(cert.Version2, cert.Curve_CURVE25519, time.Time{}, time.Time{}, nil, nil, nil)
+	caPool := cert.NewCAPool()
+	require.NoError(t, caPool.AddCA(ca))
+	certs := map[string]*cert.CachedCertificate{}
+	for _, name := range []string{"h1", "h2", "h3"} {
+		crt, _, _, _ := cert_test.NewTestCert(cert.Version2, cert.Curve_CURVE25519, ca, caKey, name, time.Time{}, time.Time{}, []netip.Prefix{netip.MustParsePrefix("10.1.0.9/16")}, nil, nil)
+		c, err := caPool.VerifyCertificate(time.Now(), crt)
+		require.NoError(t, err)
+		certs[name] = c
+	}
+
 	for _, tc := range []struct {
-		port int32
+		port uint16
 		host string
 		addr netip.Addr
 		want bool
@@ -238,8 +356,10 @@ func TestRulesBuilder_sharedLocalCIDR(t *testing.T) {
 		{3, "h3", h1Extra, false},
 		{3, "h3", h1Extra6, false},
 		{3, "h3", h2Own, false},
+		// Only h3 is allowed on port 3.
+		{3, "h1", vpn1, false},
 	} {
-		lr := rules.In.protos[iputil.IPProtocolTCP][tc.port].Any.Hosts[tc.host]
-		assert.Equal(t, tc.want, lr.match(&Packet{LocalAddr: tc.addr}, nil), "port %d, %s, %s", tc.port, tc.host, tc.addr)
+		p := &Packet{Protocol: iputil.IPProtocolTCP, LocalPort: tc.port, LocalAddr: tc.addr, RemoteAddr: netip.MustParseAddr("10.1.0.9")}
+		assert.Equal(t, tc.want, rules.In.Match(p, true, certs[tc.host], caPool), "port %d, %s, %s", tc.port, tc.host, tc.addr)
 	}
 }

firewall/table.go

@@ -3,7 +3,6 @@ package firewall
 import (
 	"net/netip"
 
-	"github.com/gaissmai/bart"
 	"github.com/slackhq/nebula/cert"
 	"github.com/slackhq/nebula/iputil"
 )
@@ -18,54 +17,80 @@ type Rules struct {
 	HashFNV uint32
 }
 
-// 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.
+// Table holds the rules for one direction. A packet is allowed when every clause of a single rule allows it:
+// proto AND port AND (CA SHA or CA name) AND local CIDR AND (group OR groups OR name OR remote CIDR).
+// A Table does not 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
+	// rules holds every rule for the direction. A rule's index in this slice is its id in the ruleSets.
+	rules []rule
+
+	// protos holds the index for each IP protocol number, so a packet is checked against a single index.
+	// A protocol with rules of its own has an index that also includes every proto `any` rule. The remaining
+	// protocols share an index of only the proto `any` rules: one for protocols with ports and one for
+	// protocols without; see hasPorts. An entry is nil when the protocol has no rules at all.
+	// ICMP and ICMPv6 share one index.
+	protos [256]*protoIndex
 }
 
-// Even though ports are uint16, int32 maps are faster for lookup
-// Plus we can use `-1` for fragment rules
-type portRules map[int32]*caRules
-
-// caRules and the types below it only make their maps and tables once a rule needs them, a range of ports
-// gets a copy for every port so empty ones add up
-type caRules struct {
-	Any     *remoteRules
-	CANames map[string]*remoteRules
-	CAShas  map[string]*remoteRules
+// protoIndex holds the rules that can apply to one protocol, in a set for each kind of packet.
+type protoIndex struct {
+	// hasPorts reports whether the protocol has ports; see the hasPorts function.
+	hasPorts bool
+	// byPort has a set for every port number of a protocol with ports. Port 0 is covered only by port `any`
+	// rules, since a range starts at 1.
+	byPort ruleSets
+	// packet holds the rules for a packet of a protocol without ports: the port `any` rules.
+	packet ruleSet
+	// fragment holds the rules for a fragment of any protocol: the port `any` and port `fragment` rules.
+	fragment ruleSet
 }
 
-type remoteRules struct {
-	// Any makes Hosts, Groups, and CIDR irrelevant
-	Any    *localRules
-	Hosts  map[string]*localRules
-	Groups []*groupsRule
-	// CIDR is checked with Prefix.Contains, so 0.0.0.0/0 allows any IPv4 address and ::/0 any IPv6 address,
-	// neither allows the other family
-	CIDR []cidrRule
+// hasPorts reports whether Nebula parses port numbers from packets of proto. parseV4 and parseV6 in outside.go
+// must agree. A rule with a port or a port range only applies to these protocols.
+func hasPorts(proto uint8) bool {
+	switch proto {
+	case iputil.IPProtocolTCP, iputil.IPProtocolUDP, iputil.IPProtocolUDPLite, iputil.IPProtocolDCCP, iputil.IPProtocolSCTP:
+		return true
+	}
+	return false
 }
 
-type groupsRule struct {
-	Groups    []string
-	LocalCIDR *localRules
+// whichMatch returns the set of rules whose proto and port clauses allow p.
+func (pi *protoIndex) whichMatch(p *Packet, incoming bool) ruleSet {
+	switch {
+	case p.Fragment:
+		// The ports of a fragmented packet are in its first fragment, and this is a later one.
+		return pi.fragment
+	case !pi.hasPorts:
+		// The port fields are zero, or for ICMP the identifier, which is only for connection tracking.
+		return pi.packet
+	case incoming:
+		return pi.byPort.at(int(p.LocalPort))
+	default:
+		return pi.byPort.at(int(p.RemotePort))
+	}
 }
 
-type cidrRule struct {
-	// Prefix is masked, so a cidr written two ways is one cidrRule
-	Prefix    netip.Prefix
-	LocalCIDR *localRules
-}
+// rule is a parsed rule. A packet is allowed by the rule when its port, certificate, and address clauses all pass.
+type rule struct {
+	// startPort and endPort are the port clause. A startPort of PortAny covers every port, every packet of a
+	// protocol without ports, and every fragment. PortFragment covers only fragments. A range covers the ports
+	// in it, so it never applies to a protocol without ports.
+	startPort, endPort int32
 
-type localRules struct {
-	Any bool
-	// LocalCIDR is always set when Any isn't. It's shared with other localRules, so it's never changed.
-	LocalCIDR *bart.Lite
+	// caSha and caName pass a certificate issued by either CA. When neither is set, any CA passes.
+	caSha, caName string
+
+	// remoteAny makes host, groups, and cidr irrelevant. Otherwise, the peer passes when any one of them matches.
+	remoteAny bool
+	host      string
+	groups    []string
+	cidr      netip.Prefix // Invalid when the rule has no cidr.
+
+	// localAny allows any local address. Otherwise, local must contain the address. Neither is set for a rule
+	// without a local cidr until the Table is built and the rule gets the default; see withDefaultLocal.
+	localAny bool
+	local    []netip.Prefix
 }
 
 // Match reports whether a rule for the direction p is going allows it
@@ -79,101 +104,33 @@ func (r *Rules) Match(p *Packet, incoming bool, c *cert.CachedCertificate, caPoo
 
 // 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 {
+	proto := t.protos[p.Protocol]
+	if proto == nil {
+		// The protocol has no rules of its own, and there are no proto `any` rules.
 		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)
-	}
+	// The index yields every rule whose proto and port clauses allow p. The packet is allowed when the
+	// remaining clauses of any of those rules allow it as well.
+	return proto.whichMatch(p, incoming).any(func(id int) bool {
+		return t.rules[id].match(p, c, caPool)
+	})
+}
 
-	var port int32
+// match reports whether r allows p from the peer with certificate c. The port clause is not checked here; the
+// protoIndex has already done so.
+func (r *rule) match(p *Packet, c *cert.CachedCertificate, caPool *cert.CAPool) bool {
+	return r.matchLocal(p.LocalAddr) && r.matchRemote(p.RemoteAddr, c) && r.matchCA(c, caPool)
+}
 
-	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) {
+// matchLocal reports whether r allows the local address addr.
+func (r *rule) matchLocal(addr netip.Addr) bool {
+	if r.localAny {
 		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 i := range rr.CIDR {
-		if rr.CIDR[i].Prefix.Contains(p.RemoteAddr) && rr.CIDR[i].LocalCIDR.match(p, c) {
+	for _, prefix := range r.local {
+		if prefix.Contains(addr) {
 			return true
 		}
 	}
@@ -181,14 +138,52 @@ func (rr *remoteRules) match(p *Packet, c *cert.CachedCertificate) bool {
 	return false
 }
 
-func (lr *localRules) match(p *Packet, c *cert.CachedCertificate) bool {
-	if lr == nil {
-		return false
-	}
-
-	if lr.Any {
+// matchRemote reports whether r allows the peer with certificate c, sending from addr. The cidr is checked with
+// Prefix.Contains, so 0.0.0.0/0 allows any IPv4 address and ::/0 allows any IPv6 address; neither allows the
+// other family.
+func (r *rule) matchRemote(addr netip.Addr, c *cert.CachedCertificate) bool {
+	if r.remoteAny {
 		return true
 	}
 
-	return lr.LocalCIDR.Contains(p.LocalAddr)
+	if r.cidr.IsValid() && r.cidr.Contains(addr) {
+		return true
+	}
+
+	if r.host != "" && r.host == c.Certificate.Name() {
+		return true
+	}
+
+	// Every group must be present.
+	if len(r.groups) == 0 {
+		return false
+	}
+	for _, g := range r.groups {
+		if _, ok := c.InvertedGroups[g]; !ok {
+			return false
+		}
+	}
+
+	return true
+}
+
+// matchCA reports whether c was issued by a CA that r allows.
+func (r *rule) matchCA(c *cert.CachedCertificate, caPool *cert.CAPool) bool {
+	if r.caSha == "" && r.caName == "" {
+		return true
+	}
+
+	if r.caSha != "" && r.caSha == c.Certificate.Issuer() {
+		return true
+	}
+
+	if r.caName != "" {
+		s, err := caPool.GetCAForCert(c.Certificate)
+		if err != nil {
+			return false
+		}
+		return r.caName == s.Certificate.Name()
+	}
+
+	return false
 }