stuff

firewall.go

@@ -45,6 +45,10 @@ type Firewall struct {
 	InRules  *FirewallTable
 	OutRules *FirewallTable
 
+	// inBuilder and outBuilder collect the rules from AddRule until buildRules turns them into InRules and OutRules
+	inBuilder  *FirewallTableBuilder
+	outBuilder *FirewallTableBuilder
+
 	InboundSendReject  bool
 	OutboundSendReject bool
 
@@ -88,89 +92,128 @@ 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)
+// A FirewallTable doesn't change once built, see FirewallTableBuilder.
 type FirewallTable 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 AnyProto.
+	// 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[firewall.ProtoAny]. nil when a protocol has no rules at all.
 	// ICMP and ICMPv6 share one set of rules.
 	Protos [256]firewallPort
+}
 
-	// AnyProto holds the proto `any` rules, nil until there is one
-	AnyProto firewallPort
-
-	// own marks the protocols in Protos with rules of their own
-	own [256]bool
-
-	// anyRules are the proto `any` rules, kept to copy into protocols that get rules of their own later
+// FirewallTableBuilder collects the rules for a FirewallTable, build turns them into one
+type FirewallTableBuilder struct {
 	anyRules []firewallPortRule
+	// protoRules holds the rules for each protocol besides proto `any`, ICMPv6 rules are kept with ICMP
+	protoRules [256][]firewallPortRule
 }
 
-// firewallPortRule holds the arguments to firewallPort.addRule
+// firewallPortRule is a rule for a firewallPort, parsed so adding it can't fail
 type firewallPortRule struct {
-	startPort, endPort                   int32
-	groups                               []string
-	host, cidr, localCidr, caName, caSha string
+	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 not valid when the rule doesn't have one, which means the vpn networks if there are unsafe networks
+	localCIDR    netip.Prefix
+	anyLocalCIDR bool
 }
 
-func (r firewallPortRule) addTo(f *Firewall, fp firewallPort) error {
-	return fp.addRule(f, r.startPort, r.endPort, r.groups, r.host, r.cidr, r.localCidr, r.caName, r.caSha)
-}
-
-func newFirewallTable() *FirewallTable {
-	return &FirewallTable{}
-}
-
-// addRule adds r to the rules for proto, or to the rules for every protocol when proto is firewall.ProtoAny
-func (ft *FirewallTable) addRule(f *Firewall, proto uint8, r firewallPortRule) error {
-	if proto == firewall.ProtoAny {
-		return ft.addAnyProtoRule(f, r)
+func parseFirewallPortRule(startPort, endPort int32, groups []string, host, cidr, localCidr, caName, caSha string) (firewallPortRule, error) {
+	if startPort > endPort {
+		return firewallPortRule{}, fmt.Errorf("start port was lower than end port")
 	}
 
-	if !ft.own[proto] {
-		fp := firewallPort{}
-		for _, ar := range ft.anyRules {
-			if err := ar.addTo(f, fp); err != nil {
-				return err
-			}
-		}
+	r := firewallPortRule{
+		startPort: startPort,
+		endPort:   endPort,
+		groups:    groups,
+		host:      host,
+		caName:    caName,
+		caSha:     caSha,
+	}
 
-		ft.Protos[proto] = fp
-		ft.own[proto] = true
-		switch proto {
-		case iputil.IPProtocolICMP:
-			ft.Protos[iputil.IPProtocolICMPv6] = fp
-			ft.own[iputil.IPProtocolICMPv6] = true
-		case iputil.IPProtocolICMPv6:
-			ft.Protos[iputil.IPProtocolICMP] = fp
-			ft.own[iputil.IPProtocolICMP] = true
+	var err error
+	r.cidr, r.anyCIDR, err = parseFirewallCIDR(cidr)
+	if err != nil {
+		return firewallPortRule{}, err
+	}
+	r.localCIDR, r.anyLocalCIDR, err = parseFirewallCIDR(localCidr)
+	if err != nil {
+		return firewallPortRule{}, err
+	}
+
+	return r, nil
+}
+
+// parseFirewallCIDR parses a rule's cidr, which is empty, `any`, or a prefix
+func parseFirewallCIDR(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 firewallPortRule) 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
+}
+
+// addRule keeps r for proto, or for every protocol when proto is firewall.ProtoAny
+func (b *FirewallTableBuilder) addRule(proto uint8, r firewallPortRule) {
+	switch proto {
+	case firewall.ProtoAny:
+		b.anyRules = append(b.anyRules, r)
+	case iputil.IPProtocolICMPv6:
+		b.protoRules[iputil.IPProtocolICMP] = append(b.protoRules[iputil.IPProtocolICMP], r)
+	default:
+		b.protoRules[proto] = append(b.protoRules[proto], r)
+	}
+}
+
+func (b *FirewallTableBuilder) build(f *Firewall) *FirewallTable {
+	var anyProto firewallPort
+	if len(b.anyRules) > 0 {
+		anyProto = firewallPort{}
+		for _, r := range b.anyRules {
+			anyProto.addRule(f, r)
 		}
 	}
 
-	return r.addTo(f, ft.Protos[proto])
-}
-
-func (ft *FirewallTable) addAnyProtoRule(f *Firewall, r firewallPortRule) error {
-	if ft.AnyProto == nil {
-		ft.AnyProto = firewallPort{}
-	}
-	if err := r.addTo(f, ft.AnyProto); err != nil {
-		return err
-	}
-	ft.anyRules = append(ft.anyRules, r)
-
+	ft := &FirewallTable{}
 	for proto := range ft.Protos {
-		switch {
-		case !ft.own[proto]:
-			ft.Protos[proto] = ft.AnyProto
-		case proto == iputil.IPProtocolICMPv6:
-			// Shares its rules with ICMP, which gets the copy
-		default:
-			if err := r.addTo(f, ft.Protos[proto]); err != nil {
-				return err
-			}
+		rules := b.protoRules[proto]
+		if len(rules) == 0 {
+			ft.Protos[proto] = anyProto
+			continue
 		}
-	}
 
-	return nil
+		fp := firewallPort{}
+		for _, r := range b.anyRules {
+			fp.addRule(f, r)
+		}
+		for _, r := range rules {
+			fp.addRule(f, r)
+		}
+		ft.Protos[proto] = fp
+	}
+	ft.Protos[iputil.IPProtocolICMPv6] = ft.Protos[iputil.IPProtocolICMP]
+
+	return ft
 }
 
 type FirewallCA struct {
@@ -239,8 +282,10 @@ func NewFirewall(l *slog.Logger, tcpTimeout, UDPTimeout, defaultTimeout time.Dur
 			Conns:      make(map[firewall.Packet]*conn),
 			TimerWheel: NewTimerWheel[firewall.Packet](tmin, tmax),
 		},
-		InRules:          newFirewallTable(),
-		OutRules:         newFirewallTable(),
+		InRules:          &FirewallTable{},
+		OutRules:         &FirewallTable{},
+		inBuilder:        &FirewallTableBuilder{},
+		outBuilder:       &FirewallTableBuilder{},
 		TCPTimeout:       tcpTimeout,
 		UDPTimeout:       UDPTimeout,
 		DefaultTimeout:   defaultTimeout,
@@ -315,16 +360,22 @@ func NewFirewallFromConfig(l *slog.Logger, cs *CertState, c *config.C) (*Firewal
 		return nil, err
 	}
 
+	err = fw.buildRules()
+	if err != nil {
+		return nil, err
+	}
+
 	return fw, nil
 }
 
 // AddRule properly creates the in memory rule structure for a firewall table.
 func (f *Firewall) AddRule(incoming bool, proto uint8, startPort int32, endPort int32, groups []string, host string, cidr, localCidr, caName string, caSha string) error {
-	var ft *FirewallTable
+	b := f.outBuilder
 	if incoming {
-		ft = f.InRules
-	} else {
-		ft = f.OutRules
+		b = f.inBuilder
+	}
+	if b == nil {
+		return errors.New("firewall rules can't be added once they're built")
 	}
 
 	if proto == iputil.IPProtocolICMP || proto == iputil.IPProtocolICMPv6 {
@@ -336,6 +387,11 @@ func (f *Firewall) AddRule(incoming bool, proto uint8, startPort int32, endPort
 		endPort = firewall.PortAny
 	}
 
+	r, err := parseFirewallPortRule(startPort, endPort, groups, host, cidr, localCidr, caName, caSha)
+	if err != nil {
+		return err
+	}
+
 	// We need this rule string because we generate a hash. Removing this will break firewall reload.
 	ruleString := fmt.Sprintf(
 		"incoming: %v, proto: %v, startPort: %v, endPort: %v, groups: %v, host: %v, ip: %v, localIp: %v, caName: %v, caSha: %s",
@@ -351,16 +407,19 @@ func (f *Firewall) AddRule(incoming bool, proto uint8, startPort int32, endPort
 		"firewallRule", m{"direction": direction, "proto": proto, "startPort": startPort, "endPort": endPort, "groups": groups, "host": host, "cidr": cidr, "localCidr": localCidr, "caName": caName, "caSha": caSha},
 	)
 
-	return ft.addRule(f, proto, firewallPortRule{
-		startPort: startPort,
-		endPort:   endPort,
-		groups:    groups,
-		host:      host,
-		cidr:      cidr,
-		localCidr: localCidr,
-		caName:    caName,
-		caSha:     caSha,
-	})
+	b.addRule(proto, r)
+	return nil
+}
+
+// buildRules turns the rules from AddRule into InRules and OutRules, no rules can be added after
+func (f *Firewall) buildRules() error {
+	if f.inBuilder == nil || f.outBuilder == nil {
+		return errors.New("firewall rules are already built")
+	}
+
+	f.InRules, f.OutRules = f.inBuilder.build(f), f.outBuilder.build(f)
+	f.inBuilder, f.outBuilder = nil, nil
+	return nil
 }
 
 // GetRuleHash returns a hash representation of all inbound and outbound rules
@@ -708,12 +767,8 @@ func (ft *FirewallTable) match(p *firewall.Packet, incoming bool, c *cert.Cached
 	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 {
-	if startPort > endPort {
-		return fmt.Errorf("start port was lower than end port")
-	}
-
-	for i := startPort; i <= endPort; i++ {
+func (fp firewallPort) addRule(f *Firewall, r firewallPortRule) {
+	for i := r.startPort; i <= r.endPort; i++ {
 		if _, ok := fp[i]; !ok {
 			fp[i] = &FirewallCA{
 				CANames: make(map[string]*FirewallRule),
@@ -721,12 +776,8 @@ func (fp firewallPort) addRule(f *Firewall, startPort int32, endPort int32, grou
 			}
 		}
 
-		if err := fp[i].addRule(f, groups, host, cidr, localCidr, caName, caSha); err != nil {
-			return err
-		}
+		fp[i].addRule(f, r)
 	}
-
-	return nil
 }
 
 func (fp firewallPort) match(p *firewall.Packet, incoming bool, c *cert.CachedCertificate, caPool *cert.CAPool) bool {
@@ -760,7 +811,7 @@ func (fp firewallPort) match(p *firewall.Packet, incoming bool, c *cert.CachedCe
 	return fp[firewall.PortAny].match(p, c, caPool)
 }
 
-func (fc *FirewallCA) addRule(f *Firewall, groups []string, host string, cidr, localCidr, caName, caSha string) error {
+func (fc *FirewallCA) addRule(f *Firewall, r firewallPortRule) {
 	fr := func() *FirewallRule {
 		return &FirewallRule{
 			Hosts:  make(map[string]*firewallLocalCIDR),
@@ -769,35 +820,28 @@ func (fc *FirewallCA) addRule(f *Firewall, groups []string, host string, cidr, l
 		}
 	}
 
-	if caSha == "" && caName == "" {
+	if r.caSha == "" && r.caName == "" {
 		if fc.Any == nil {
 			fc.Any = fr()
 		}
 
-		return fc.Any.addRule(f, groups, host, cidr, localCidr)
+		fc.Any.addRule(f, r)
+		return
 	}
 
-	if caSha != "" {
-		if _, ok := fc.CAShas[caSha]; !ok {
-			fc.CAShas[caSha] = fr()
-		}
-		err := fc.CAShas[caSha].addRule(f, groups, host, cidr, localCidr)
-		if err != nil {
-			return err
+	if r.caSha != "" {
+		if _, ok := fc.CAShas[r.caSha]; !ok {
+			fc.CAShas[r.caSha] = fr()
 		}
+		fc.CAShas[r.caSha].addRule(f, r)
 	}
 
-	if caName != "" {
-		if _, ok := fc.CANames[caName]; !ok {
-			fc.CANames[caName] = fr()
-		}
-		err := fc.CANames[caName].addRule(f, groups, host, cidr, localCidr)
-		if err != nil {
-			return err
+	if r.caName != "" {
+		if _, ok := fc.CANames[r.caName]; !ok {
+			fc.CANames[r.caName] = fr()
 		}
+		fc.CANames[r.caName].addRule(f, r)
 	}
-
-	return nil
 }
 
 func (fc *FirewallCA) match(p *firewall.Packet, c *cert.CachedCertificate, caPool *cert.CAPool) bool {
@@ -823,83 +867,49 @@ func (fc *FirewallCA) match(p *firewall.Packet, c *cert.CachedCertificate, caPoo
 	return fc.CANames[s.Certificate.Name()].match(p, c)
 }
 
-func (fr *FirewallRule) addRule(f *Firewall, groups []string, host, cidr, localCidr string) error {
+func (fr *FirewallRule) addRule(f *Firewall, r firewallPortRule) {
 	flc := func() *firewallLocalCIDR {
 		return &firewallLocalCIDR{
 			LocalCIDR: new(bart.Lite),
 		}
 	}
 
-	if fr.isAny(groups, host, cidr) {
+	if r.isAny() {
 		if fr.Any == nil {
 			fr.Any = flc()
 		}
 
-		return fr.Any.addRule(f, localCidr)
+		fr.Any.addRule(f, r)
+		return
 	}
 
-	if len(groups) > 0 {
+	if len(r.groups) > 0 {
 		nlc := flc()
-		err := nlc.addRule(f, localCidr)
-		if err != nil {
-			return err
-		}
+		nlc.addRule(f, r)
 
 		fr.Groups = append(fr.Groups, &firewallGroups{
-			Groups:    groups,
+			Groups:    r.groups,
 			LocalCIDR: nlc,
 		})
 	}
 
-	if host != "" {
-		nlc := fr.Hosts[host]
+	if r.host != "" {
+		nlc := fr.Hosts[r.host]
 		if nlc == nil {
 			nlc = flc()
 		}
-		err := nlc.addRule(f, localCidr)
-		if err != nil {
-			return err
-		}
-		fr.Hosts[host] = nlc
+		nlc.addRule(f, r)
+		fr.Hosts[r.host] = nlc
 	}
 
-	if cidr != "" {
-		c, err := netip.ParsePrefix(cidr)
-		if err != nil {
-			return err
-		}
-		nlc, _ := fr.CIDR.Get(c)
+	if r.cidr.IsValid() {
+		nlc, _ := fr.CIDR.Get(r.cidr)
 		if nlc == nil {
 			nlc = flc()
 		}
-		err = nlc.addRule(f, localCidr)
-		if err != nil {
-			return err
-		}
-		fr.CIDR.Insert(c, nlc)
+		nlc.addRule(f, r)
+		fr.CIDR.Insert(r.cidr, nlc)
 	}
-
-	return nil
-}
-
-func (fr *FirewallRule) isAny(groups []string, host string, cidr string) bool {
-	if len(groups) == 0 && host == "" && cidr == "" {
-		return true
-	}
-
-	if slices.Contains(groups, "any") {
-		return true
-	}
-
-	if host == "any" {
-		return true
-	}
-
-	if cidr == "any" {
-		return true
-	}
-
-	return false
 }
 
 func (fr *FirewallRule) match(p *firewall.Packet, c *cert.CachedCertificate) bool {
@@ -947,31 +957,25 @@ func (fr *FirewallRule) match(p *firewall.Packet, c *cert.CachedCertificate) boo
 	return false
 }
 
-func (flc *firewallLocalCIDR) addRule(f *Firewall, localCidr string) error {
-	if localCidr == "any" {
+func (flc *firewallLocalCIDR) addRule(f *Firewall, r firewallPortRule) {
+	if r.anyLocalCIDR {
 		flc.Any = true
-		return nil
+		return
 	}
 
-	if localCidr == "" {
+	if !r.localCIDR.IsValid() {
 		if len(f.unsafeNetworks) == 0 || f.defaultLocalCIDRAny {
 			flc.Any = true
-			return nil
+			return
 		}
 
 		for _, network := range f.assignedNetworks {
 			flc.LocalCIDR.Insert(network)
 		}
-		return nil
-
+		return
 	}
 
-	c, err := netip.ParsePrefix(localCidr)
-	if err != nil {
-		return err
-	}
-	flc.LocalCIDR.Insert(c)
-	return nil
+	flc.LocalCIDR.Insert(r.localCIDR)
 }
 
 func (flc *firewallLocalCIDR) match(p *firewall.Packet, c *cert.CachedCertificate) bool {

firewall_test.go

@@ -82,6 +82,7 @@ func TestFirewall_AddRule(t *testing.T) {
 	require.NoError(t, err)
 
 	require.NoError(t, fw.AddRule(true, iputil.IPProtocolTCP, 1, 1, []string{}, "", "", "", "", ""))
+	require.NoError(t, fw.buildRules())
 	// An empty rule is any
 	assert.True(t, fw.InRules.Protos[iputil.IPProtocolTCP][1].Any.Any.Any)
 	assert.Empty(t, fw.InRules.Protos[iputil.IPProtocolTCP][1].Any.Groups)
@@ -89,73 +90,84 @@ 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"}, "", "", "", "", ""))
+	require.NoError(t, fw.buildRules())
 	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", "", "", "", ""))
+	require.NoError(t, fw.AddRule(true, iputil.IPProtocolICMPv6, 1, 1, []string{}, "h2", "", "", "", ""))
+	require.NoError(t, fw.buildRules())
 	//no matter what port is given for icmp, it should end up as "any"
 	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
+	require.NoError(t, fw.buildRules())
 	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(), "", "", ""))
-	assert.Nil(t, fw.OutRules.AnyProto[1].Any.Any)
-	_, ok := fw.OutRules.AnyProto[1].Any.CIDR.Get(ti)
+	require.NoError(t, fw.buildRules())
+	assert.Nil(t, fw.OutRules.Protos[firewall.ProtoAny][1].Any.Any)
+	_, ok := fw.OutRules.Protos[firewall.ProtoAny][1].Any.CIDR.Get(ti)
 	assert.True(t, ok)
 
 	fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c)
 	require.NoError(t, fw.AddRule(false, firewall.ProtoAny, 1, 1, []string{}, "", ti6.String(), "", "", ""))
-	assert.Nil(t, fw.OutRules.AnyProto[1].Any.Any)
-	_, ok = fw.OutRules.AnyProto[1].Any.CIDR.Get(ti6)
+	require.NoError(t, fw.buildRules())
+	assert.Nil(t, fw.OutRules.Protos[firewall.ProtoAny][1].Any.Any)
+	_, ok = fw.OutRules.Protos[firewall.ProtoAny][1].Any.CIDR.Get(ti6)
 	assert.True(t, ok)
 
 	fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c)
 	require.NoError(t, fw.AddRule(false, firewall.ProtoAny, 1, 1, []string{}, "", "", ti.String(), "", ""))
-	assert.NotNil(t, fw.OutRules.AnyProto[1].Any.Any)
-	ok = fw.OutRules.AnyProto[1].Any.Any.LocalCIDR.Get(ti)
+	require.NoError(t, fw.buildRules())
+	assert.NotNil(t, fw.OutRules.Protos[firewall.ProtoAny][1].Any.Any)
+	ok = fw.OutRules.Protos[firewall.ProtoAny][1].Any.Any.LocalCIDR.Get(ti)
 	assert.True(t, ok)
 
 	fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c)
 	require.NoError(t, fw.AddRule(false, firewall.ProtoAny, 1, 1, []string{}, "", "", ti6.String(), "", ""))
-	assert.NotNil(t, fw.OutRules.AnyProto[1].Any.Any)
-	ok = fw.OutRules.AnyProto[1].Any.Any.LocalCIDR.Get(ti6)
+	require.NoError(t, fw.buildRules())
+	assert.NotNil(t, fw.OutRules.Protos[firewall.ProtoAny][1].Any.Any)
+	ok = fw.OutRules.Protos[firewall.ProtoAny][1].Any.Any.LocalCIDR.Get(ti6)
 	assert.True(t, ok)
 
 	fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c)
 	require.NoError(t, fw.AddRule(true, iputil.IPProtocolUDP, 1, 1, []string{"g1"}, "", "", "", "ca-name", ""))
+	require.NoError(t, fw.buildRules())
 	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"))
+	require.NoError(t, fw.buildRules())
 	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", "", "", "", ""))
-	assert.True(t, fw.OutRules.AnyProto[0].Any.Any.Any)
+	require.NoError(t, fw.buildRules())
+	assert.True(t, fw.OutRules.Protos[firewall.ProtoAny][0].Any.Any.Any)
 
 	fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c)
 	anyIp, err := netip.ParsePrefix("0.0.0.0/0")
 	require.NoError(t, err)
 
 	require.NoError(t, fw.AddRule(false, firewall.ProtoAny, 0, 0, []string{}, "", anyIp.String(), "", "", ""))
-	assert.Nil(t, fw.OutRules.AnyProto[0].Any.Any)
-	table, ok := fw.OutRules.AnyProto[0].Any.CIDR.Lookup(netip.MustParseAddr("1.1.1.1"))
+	require.NoError(t, fw.buildRules())
+	assert.Nil(t, fw.OutRules.Protos[firewall.ProtoAny][0].Any.Any)
+	table, ok := fw.OutRules.Protos[firewall.ProtoAny][0].Any.CIDR.Lookup(netip.MustParseAddr("1.1.1.1"))
 	assert.True(t, table.Any)
-	table, ok = fw.OutRules.AnyProto[0].Any.CIDR.Lookup(netip.MustParseAddr("9::9"))
+	table, ok = fw.OutRules.Protos[firewall.ProtoAny][0].Any.CIDR.Lookup(netip.MustParseAddr("9::9"))
 	assert.False(t, ok)
 
 	fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c)
@@ -163,40 +175,58 @@ func TestFirewall_AddRule(t *testing.T) {
 	require.NoError(t, err)
 
 	require.NoError(t, fw.AddRule(false, firewall.ProtoAny, 0, 0, []string{}, "", anyIp6.String(), "", "", ""))
-	assert.Nil(t, fw.OutRules.AnyProto[0].Any.Any)
-	table, ok = fw.OutRules.AnyProto[0].Any.CIDR.Lookup(netip.MustParseAddr("9::9"))
+	require.NoError(t, fw.buildRules())
+	assert.Nil(t, fw.OutRules.Protos[firewall.ProtoAny][0].Any.Any)
+	table, ok = fw.OutRules.Protos[firewall.ProtoAny][0].Any.CIDR.Lookup(netip.MustParseAddr("9::9"))
 	assert.True(t, table.Any)
-	table, ok = fw.OutRules.AnyProto[0].Any.CIDR.Lookup(netip.MustParseAddr("1.1.1.1"))
+	table, ok = fw.OutRules.Protos[firewall.ProtoAny][0].Any.CIDR.Lookup(netip.MustParseAddr("1.1.1.1"))
 	assert.False(t, ok)
 
 	fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c)
 	require.NoError(t, fw.AddRule(false, firewall.ProtoAny, 0, 0, []string{}, "", "any", "", "", ""))
-	assert.True(t, fw.OutRules.AnyProto[0].Any.Any.Any)
+	require.NoError(t, fw.buildRules())
+	assert.True(t, fw.OutRules.Protos[firewall.ProtoAny][0].Any.Any.Any)
 
 	fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c)
 	require.NoError(t, fw.AddRule(false, firewall.ProtoAny, 0, 0, []string{}, "", "", anyIp.String(), "", ""))
-	assert.False(t, fw.OutRules.AnyProto[0].Any.Any.Any)
-	assert.True(t, fw.OutRules.AnyProto[0].Any.Any.LocalCIDR.Lookup(netip.MustParseAddr("1.1.1.1")))
-	assert.False(t, fw.OutRules.AnyProto[0].Any.Any.LocalCIDR.Lookup(netip.MustParseAddr("9::9")))
+	require.NoError(t, fw.buildRules())
+	assert.False(t, fw.OutRules.Protos[firewall.ProtoAny][0].Any.Any.Any)
+	assert.True(t, fw.OutRules.Protos[firewall.ProtoAny][0].Any.Any.LocalCIDR.Lookup(netip.MustParseAddr("1.1.1.1")))
+	assert.False(t, fw.OutRules.Protos[firewall.ProtoAny][0].Any.Any.LocalCIDR.Lookup(netip.MustParseAddr("9::9")))
 
 	fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c)
 	require.NoError(t, fw.AddRule(false, firewall.ProtoAny, 0, 0, []string{}, "", "", anyIp6.String(), "", ""))
-	assert.False(t, fw.OutRules.AnyProto[0].Any.Any.Any)
-	assert.True(t, fw.OutRules.AnyProto[0].Any.Any.LocalCIDR.Lookup(netip.MustParseAddr("9::9")))
-	assert.False(t, fw.OutRules.AnyProto[0].Any.Any.LocalCIDR.Lookup(netip.MustParseAddr("1.1.1.1")))
+	require.NoError(t, fw.buildRules())
+	assert.False(t, fw.OutRules.Protos[firewall.ProtoAny][0].Any.Any.Any)
+	assert.True(t, fw.OutRules.Protos[firewall.ProtoAny][0].Any.Any.LocalCIDR.Lookup(netip.MustParseAddr("9::9")))
+	assert.False(t, fw.OutRules.Protos[firewall.ProtoAny][0].Any.Any.LocalCIDR.Lookup(netip.MustParseAddr("1.1.1.1")))
 
 	fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c)
 	require.NoError(t, fw.AddRule(false, firewall.ProtoAny, 0, 0, []string{}, "", "", "any", "", ""))
-	assert.True(t, fw.OutRules.AnyProto[0].Any.Any.Any)
+	require.NoError(t, fw.buildRules())
+	assert.True(t, fw.OutRules.Protos[firewall.ProtoAny][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{}, "", "", "", "", ""))
+	require.NoError(t, fw.buildRules())
 	assert.True(t, fw.InRules.Protos[math.MaxUint8][firewall.PortAny].Any.Any.Any)
 
-	// Test error conditions
+	// Test error conditions, a bad rule is reported when it's added
 	fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c)
 	require.Error(t, fw.AddRule(true, firewall.ProtoAny, 10, 0, []string{}, "", "", "", "", ""))
+	require.Error(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{}, "", "junk", "", "", ""))
+	require.Error(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{}, "", "", "junk", "", ""))
+	require.NoError(t, fw.buildRules())
+
+	// Rules take effect once built, and can't change after
+	fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c)
+	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", ""))
+	assert.Nil(t, fw.InRules.Protos[iputil.IPProtocolTCP])
+	require.NoError(t, fw.buildRules())
+	assert.NotNil(t, fw.InRules.Protos[iputil.IPProtocolTCP])
+	require.Error(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", ""))
+	require.Error(t, fw.buildRules())
 }
 
 func TestFirewall_Drop(t *testing.T) {
@@ -232,6 +262,7 @@ func TestFirewall_Drop(t *testing.T) {
 
 	fw := NewFirewall(l, time.Second, time.Minute, time.Hour, &c)
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", ""))
+	require.NoError(t, fw.buildRules())
 	cp := cert.NewCAPool()
 
 	// Drop outbound
@@ -252,12 +283,14 @@ func TestFirewall_Drop(t *testing.T) {
 	fw = NewFirewall(l, time.Second, time.Minute, time.Hour, &c)
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"nope"}, "", "", "", "", "signer-shasum"))
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"default-group"}, "", "", "", "", "signer-shasum-bad"))
+	require.NoError(t, fw.buildRules())
 	assert.Equal(t, fw.Drop(p, true, &h, cp, nil), ErrNoMatchingRule)
 
 	// test caSha doesn't drop on match
 	fw = NewFirewall(l, time.Second, time.Minute, time.Hour, &c)
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"nope"}, "", "", "", "", "signer-shasum-bad"))
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"default-group"}, "", "", "", "", "signer-shasum"))
+	require.NoError(t, fw.buildRules())
 	require.NoError(t, fw.Drop(p, true, &h, cp, nil))
 
 	// ensure ca name doesn't get in the way of group checks
@@ -265,6 +298,7 @@ func TestFirewall_Drop(t *testing.T) {
 	fw = NewFirewall(l, time.Second, time.Minute, time.Hour, &c)
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"nope"}, "", "", "", "ca-good", ""))
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"default-group"}, "", "", "", "ca-good-bad", ""))
+	require.NoError(t, fw.buildRules())
 	assert.Equal(t, fw.Drop(p, true, &h, cp, nil), ErrNoMatchingRule)
 
 	// test caName doesn't drop on match
@@ -272,6 +306,7 @@ func TestFirewall_Drop(t *testing.T) {
 	fw = NewFirewall(l, time.Second, time.Minute, time.Hour, &c)
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"nope"}, "", "", "", "ca-good-bad", ""))
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"default-group"}, "", "", "", "ca-good", ""))
+	require.NoError(t, fw.buildRules())
 	require.NoError(t, fw.Drop(p, true, &h, cp, nil))
 }
 
@@ -310,6 +345,7 @@ func TestFirewall_DropV6(t *testing.T) {
 
 	fw := NewFirewall(l, time.Second, time.Minute, time.Hour, &c)
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", ""))
+	require.NoError(t, fw.buildRules())
 	cp := cert.NewCAPool()
 
 	// Drop outbound
@@ -330,12 +366,14 @@ func TestFirewall_DropV6(t *testing.T) {
 	fw = NewFirewall(l, time.Second, time.Minute, time.Hour, &c)
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"nope"}, "", "", "", "", "signer-shasum"))
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"default-group"}, "", "", "", "", "signer-shasum-bad"))
+	require.NoError(t, fw.buildRules())
 	assert.Equal(t, fw.Drop(p, true, &h, cp, nil), ErrNoMatchingRule)
 
 	// test caSha doesn't drop on match
 	fw = NewFirewall(l, time.Second, time.Minute, time.Hour, &c)
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"nope"}, "", "", "", "", "signer-shasum-bad"))
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"default-group"}, "", "", "", "", "signer-shasum"))
+	require.NoError(t, fw.buildRules())
 	require.NoError(t, fw.Drop(p, true, &h, cp, nil))
 
 	// ensure ca name doesn't get in the way of group checks
@@ -343,6 +381,7 @@ func TestFirewall_DropV6(t *testing.T) {
 	fw = NewFirewall(l, time.Second, time.Minute, time.Hour, &c)
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"nope"}, "", "", "", "ca-good", ""))
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"default-group"}, "", "", "", "ca-good-bad", ""))
+	require.NoError(t, fw.buildRules())
 	assert.Equal(t, fw.Drop(p, true, &h, cp, nil), ErrNoMatchingRule)
 
 	// test caName doesn't drop on match
@@ -350,20 +389,22 @@ func TestFirewall_DropV6(t *testing.T) {
 	fw = NewFirewall(l, time.Second, time.Minute, time.Hour, &c)
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"nope"}, "", "", "", "ca-good-bad", ""))
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"default-group"}, "", "", "", "ca-good", ""))
+	require.NoError(t, fw.buildRules())
 	require.NoError(t, fw.Drop(p, true, &h, cp, nil))
 }
 
 func BenchmarkFirewallTable_match(b *testing.B) {
 	f := &Firewall{}
-	ft := FirewallTable{}
+	ftb := &FirewallTableBuilder{}
 
 	pfix := netip.MustParsePrefix("172.1.1.1/32")
-	_ = ft.addRule(f, iputil.IPProtocolTCP, firewallPortRule{startPort: 10, endPort: 10, groups: []string{"good-group"}, host: "good-host", cidr: pfix.String()})
-	_ = ft.addRule(f, iputil.IPProtocolTCP, firewallPortRule{startPort: 100, endPort: 100, groups: []string{"good-group"}, host: "good-host", localCidr: pfix.String()})
+	ftb.addRule(iputil.IPProtocolTCP, firewallPortRule{startPort: 10, endPort: 10, groups: []string{"good-group"}, host: "good-host", cidr: pfix})
+	ftb.addRule(iputil.IPProtocolTCP, firewallPortRule{startPort: 100, endPort: 100, groups: []string{"good-group"}, host: "good-host", localCIDR: pfix})
 
 	pfix6 := netip.MustParsePrefix("fd11::11/128")
-	_ = ft.addRule(f, iputil.IPProtocolTCP, firewallPortRule{startPort: 10, endPort: 10, groups: []string{"good-group"}, host: "good-host", cidr: pfix6.String()})
-	_ = ft.addRule(f, iputil.IPProtocolTCP, firewallPortRule{startPort: 100, endPort: 100, groups: []string{"good-group"}, host: "good-host", localCidr: pfix6.String()})
+	ftb.addRule(iputil.IPProtocolTCP, firewallPortRule{startPort: 10, endPort: 10, groups: []string{"good-group"}, host: "good-host", cidr: pfix6})
+	ftb.addRule(iputil.IPProtocolTCP, firewallPortRule{startPort: 100, endPort: 100, groups: []string{"good-group"}, host: "good-host", localCIDR: pfix6})
+	ft := ftb.build(f)
 	cp := cert.NewCAPool()
 
 	b.Run("fail on proto", func(b *testing.B) {
@@ -552,6 +593,7 @@ func TestFirewall_Drop2(t *testing.T) {
 
 	fw := NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate)
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"default-group", "test-group"}, "", "", "", "", ""))
+	require.NoError(t, fw.buildRules())
 	cp := cert.NewCAPool()
 
 	// h1/c1 lacks the proper groups
@@ -632,6 +674,7 @@ func TestFirewall_Drop3(t *testing.T) {
 	fw := NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate)
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 1, 1, []string{}, "host1", "", "", "", ""))
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 1, 1, []string{}, "", "", "", "", "signer-sha"))
+	require.NoError(t, fw.buildRules())
 	cp := cert.NewCAPool()
 
 	// c1 should pass because host match
@@ -646,6 +689,7 @@ func TestFirewall_Drop3(t *testing.T) {
 	// Test a remote address match
 	fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate)
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 1, 1, []string{}, "", "1.2.3.4/24", "", "", ""))
+	require.NoError(t, fw.buildRules())
 	require.NoError(t, fw.Drop(p, true, &h1, cp, nil))
 }
 
@@ -683,6 +727,7 @@ func TestFirewall_Drop3V6(t *testing.T) {
 	fw := NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate)
 	cp := cert.NewCAPool()
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 1, 1, []string{}, "", "fd12::34/120", "", "", ""))
+	require.NoError(t, fw.buildRules())
 	require.NoError(t, fw.Drop(p, true, &h, cp, nil))
 }
 
@@ -721,6 +766,7 @@ func TestFirewall_DropConntrackReload(t *testing.T) {
 
 	fw := NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate)
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", ""))
+	require.NoError(t, fw.buildRules())
 	cp := cert.NewCAPool()
 
 	// Drop outbound
@@ -734,6 +780,7 @@ func TestFirewall_DropConntrackReload(t *testing.T) {
 	oldFw := fw
 	fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate)
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 10, 10, []string{"any"}, "", "", "", "", ""))
+	require.NoError(t, fw.buildRules())
 	fw.Conntrack = oldFw.Conntrack
 	fw.rulesVersion = oldFw.rulesVersion + 1
 
@@ -743,6 +790,7 @@ func TestFirewall_DropConntrackReload(t *testing.T) {
 	oldFw = fw
 	fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate)
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 11, 11, []string{"any"}, "", "", "", "", ""))
+	require.NoError(t, fw.buildRules())
 	fw.Conntrack = oldFw.Conntrack
 	fw.rulesVersion = oldFw.rulesVersion + 1
 
@@ -787,6 +835,7 @@ func TestFirewall_ICMPPortBehavior(t *testing.T) {
 	t.Run("ICMP allowed", func(t *testing.T) {
 		fw := NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate)
 		require.NoError(t, fw.AddRule(true, iputil.IPProtocolICMP, 0, 0, []string{"any"}, "", "", "", "", ""))
+		require.NoError(t, fw.buildRules())
 		t.Run("zero ports", func(t *testing.T) {
 			p := templ.Copy()
 			p.LocalPort = 0
@@ -817,6 +866,7 @@ func TestFirewall_ICMPPortBehavior(t *testing.T) {
 	t.Run("Any proto, some ports allowed", func(t *testing.T) {
 		fw := NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate)
 		require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 80, 444, []string{"any"}, "", "", "", "", ""))
+		require.NoError(t, fw.buildRules())
 		t.Run("zero ports, still blocked", func(t *testing.T) {
 			p := templ.Copy()
 			p.LocalPort = 0
@@ -859,6 +909,7 @@ func TestFirewall_ICMPPortBehavior(t *testing.T) {
 	t.Run("Any proto, any port", func(t *testing.T) {
 		fw := NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate)
 		require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", ""))
+		require.NoError(t, fw.buildRules())
 		t.Run("zero ports, allowed", func(t *testing.T) {
 			resetConntrack(fw)
 			p := templ.Copy()
@@ -923,6 +974,7 @@ func TestFirewall_ProtocolRules(t *testing.T) {
 	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"}, "", "", "", "", ""))
+		require.NoError(t, fw.buildRules())
 
 		for _, pktProto := range protos {
 			p := firewall.Packet{
@@ -953,6 +1005,7 @@ func TestFirewall_ProtocolRules(t *testing.T) {
 	// 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.buildRules())
 	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))
@@ -961,6 +1014,7 @@ func TestFirewall_ProtocolRules(t *testing.T) {
 	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.buildRules())
 		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)
@@ -979,6 +1033,7 @@ func TestFirewall_ProtocolRules(t *testing.T) {
 		if !anyFirst {
 			addAny()
 		}
+		require.NoError(t, fw.buildRules())
 
 		for _, tc := range []struct {
 			proto uint8
@@ -1010,6 +1065,7 @@ func TestFirewall_ProtocolRules(t *testing.T) {
 		if !anyFirst {
 			addAny()
 		}
+		require.NoError(t, fw.buildRules())
 
 		require.NoError(t, fw.Drop(pkt(iputil.IPProtocolICMP), true, &h, cp, nil), "anyFirst %v", anyFirst)
 		require.NoError(t, fw.Drop(pkt(iputil.IPProtocolICMPv6), true, &h, cp, nil), "anyFirst %v", anyFirst)
@@ -1019,9 +1075,14 @@ func TestFirewall_ProtocolRules(t *testing.T) {
 	// Protocols without rules of their own have no rules to check until there is a proto `any` rule to share
 	fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate)
 	require.NoError(t, fw.AddRule(true, iputil.IPProtocolTCP, 22, 22, []string{"any"}, "", "", "", "", ""))
+	require.NoError(t, fw.buildRules())
 	assert.Nil(t, fw.InRules.Protos[47])
 	assert.Equal(t, ErrNoMatchingRule, fw.Drop(pkt(47), true, &h, cp, nil))
+
+	fw = NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate)
+	require.NoError(t, fw.AddRule(true, iputil.IPProtocolTCP, 22, 22, []string{"any"}, "", "", "", "", ""))
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, firewall.PortAny, firewall.PortAny, []string{"any"}, "", "", "", "", ""))
+	require.NoError(t, fw.buildRules())
 	require.NoError(t, fw.Drop(pkt(47), true, &h, cp, nil))
 }
 
@@ -1056,6 +1117,7 @@ func TestFirewall_DropIPSpoofing(t *testing.T) {
 	fw := NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate)
 
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 1, 1, []string{}, "", "", "", "", ""))
+	require.NoError(t, fw.buildRules())
 	cp := cert.NewCAPool()
 
 	// Packet spoofed by `c1`. Note that the remote addr is not a valid one.
@@ -1108,6 +1170,7 @@ func TestFirewall_ConntrackSourceSpoofingAcrossPeers(t *testing.T) {
 	fw := NewFirewall(l, time.Second, time.Minute, time.Hour, owner)
 	// Allow any inbound traffic that passes the cert / source-IP checks.
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", ""))
+	require.NoError(t, fw.buildRules())
 	cp := cert.NewCAPool()
 
 	flow := firewall.Packet{
@@ -1203,6 +1266,7 @@ func BenchmarkFirewallDropConntrackHit(b *testing.B) {
 		b.Run(tc.name, func(b *testing.B) {
 			fw := NewFirewall(l, time.Second, time.Minute, time.Hour, owner)
 			require.NoError(b, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", ""))
+			require.NoError(b, fw.buildRules())
 
 			// Establish the conntrack entry so every benchmarked Drop is a hit.
 			require.NoError(b, fw.Drop(flow, true, tc.host, cp, nil))
@@ -1806,6 +1870,7 @@ func newSetupFromCert(t *testing.T, l *slog.Logger, c dummyCert) testsetup {
 	}
 	fw := NewFirewall(l, time.Second, time.Minute, time.Hour, &c)
 	require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", ""))
+	require.NoError(t, fw.buildRules())
 
 	return testsetup{
 		c:                  c,
@@ -1885,9 +1950,14 @@ func TestFirewall_Drop_EnforceIPMatch(t *testing.T) {
 		tc.p.LocalAddr = netip.MustParseAddr("192.168.0.3")
 		tc.err = ErrNoMatchingRule
 		tc.Test(t, unsafeSetup.fw) //should hit firewall and bounce off
-		require.NoError(t, unsafeSetup.fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", unsafePrefix.String(), "", ""))
+
+		// The same rules plus one for the unsafe route
+		fw := NewFirewall(l, time.Second, time.Minute, time.Hour, &c)
+		require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", ""))
+		require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", unsafePrefix.String(), "", ""))
+		require.NoError(t, fw.buildRules())
 		tc.err = nil
-		tc.Test(t, unsafeSetup.fw) //should pass
+		tc.Test(t, fw) //should pass
 	})
 }