Treat only non-negated addresses as scoping interfaceless zones

internal/nftables/compiler.go

@@ -417,8 +417,8 @@ func splitAddrs(addr string) []string {
 func (c *Compiler) compileZonePair(state *FirewallState, tag, srcZone, srcAddr, dstZone, dstAddr, proto string,
 	dports, sports config.PortSpec, action config.RuleAction, logLevel string,
 	fwZone string, section config.RuleSection) error {
-	srcIfaces := c.resolveZoneInterfaces(srcZone, srcAddr != "")
-	dstIfaces := c.resolveZoneInterfaces(dstZone, dstAddr != "")
+	srcIfaces := c.resolveZoneInterfaces(srcZone, srcAddr)
+	dstIfaces := c.resolveZoneInterfaces(dstZone, dstAddr)
 	chain := c.selectChain(srcZone, dstZone, fwZone)
 
 	for _, srcIface := range srcIfaces {
@@ -474,7 +474,7 @@ func (c *Compiler) compileDNATRule(state *FirewallState, tag, srcZone, srcAddr,
 		dnatPort = uint16(p)
 	}
 
-	srcIfaces := c.resolveZoneInterfaces(srcZone, srcAddr != "")
+	srcIfaces := c.resolveZoneInterfaces(srcZone, srcAddr)
 
 	matches, err := l4Matches(proto, dports, nil)
 	if err != nil {
@@ -589,8 +589,8 @@ func (c *Compiler) compilePolicies(state *FirewallState) error {
 				}
 
 				chain := c.selectChain(sz, dz, fwZone)
-				srcIfaces := c.resolveZoneInterfaces(sz, false)
-				dstIfaces := c.resolveZoneInterfaces(dz, false)
+				srcIfaces := c.resolveZoneInterfaces(sz, "")
+				dstIfaces := c.resolveZoneInterfaces(dz, "")
 
 				for _, si := range srcIfaces {
 					for _, di := range dstIfaces {
@@ -932,15 +932,15 @@ func (c *Compiler) selectChain(srcZone, dstZone, fwZone string) string {
 	return "forward"
 }
 
-// resolveZoneInterfaces returns nil (fail closed) for a zone with no interfaces unless an address match narrows the rule.
-func (c *Compiler) resolveZoneInterfaces(zone string, addrNarrowed bool) []string {
+// resolveZoneInterfaces returns nil (fail closed) for a zone with no interfaces unless a non-negated address match narrows the rule.
+func (c *Compiler) resolveZoneInterfaces(zone, addr string) []string {
 	if z, ok := c.cfg.Zones[zone]; !ok || z.Type == config.ZoneFirewall {
 		return []string{""}
 	}
 	if ifaces := c.cfg.ZoneInterfaces(zone); len(ifaces) > 0 {
 		return ifaces
 	}
-	if addrNarrowed {
+	if addr != "" && !strings.HasPrefix(addr, "!") {
 		return []string{""}
 	}
 	if !c.warned[zone] {

internal/nftables/compiler_test.go

@@ -1,8 +1,10 @@
 package nftables
 
 import (
+	"bytes"
 	"encoding/binary"
 	"fmt"
+	"log/slog"
 	"reflect"
 	"strings"
 	"testing"
@@ -141,17 +143,17 @@ func TestCompiler_ResolveZoneInterfaces(t *testing.T) {
 	}
 	c := NewCompiler(cfg)
 
-	ifaces := c.resolveZoneInterfaces("net", false)
+	ifaces := c.resolveZoneInterfaces("net", "")
 	if len(ifaces) != 1 || ifaces[0] != "eth0" {
 		t.Errorf("resolveZoneInterfaces(net) = %v, want [eth0]", ifaces)
 	}
 
-	ifaces = c.resolveZoneInterfaces("all", false)
+	ifaces = c.resolveZoneInterfaces("all", "")
 	if len(ifaces) != 1 || ifaces[0] != "" {
 		t.Errorf("resolveZoneInterfaces(all) = %v, want [\"\"]", ifaces)
 	}
 
-	ifaces = c.resolveZoneInterfaces("fw", false)
+	ifaces = c.resolveZoneInterfaces("fw", "")
 	if len(ifaces) != 1 || ifaces[0] != "" {
 		t.Errorf("resolveZoneInterfaces(fw) = %v, want [\"\"]", ifaces)
 	}
@@ -1771,36 +1773,66 @@ func TestCompile_OutputPolicyMatchesOif(t *testing.T) {
 }
 
 func TestCompile_InterfacelessZonesFailClosed(t *testing.T) {
+	ipsec := func(c *config.Config) { c.Zones["ips"] = config.Zone{Type: config.ZoneIPSec} }
+	rule := func(dest string) func(*config.Config) {
+		return func(c *config.Config) {
+			ipsec(c)
+			c.Rules = []config.Rule{{Action: config.RuleAccept, Source: "fw", Dest: dest}}
+		}
+	}
 	tests := []struct {
-		name string
-		mod  func(*config.Config)
+		name  string
+		mod   func(*config.Config)
+		tag   string
+		want  int
+		warns []string
 	}{
 		{"ipsec zone without interface", func(c *config.Config) {
-			c.Zones["ips"] = config.Zone{Type: config.ZoneIPSec}
+			ipsec(c)
 			c.Policy = []config.Policy{{Source: "fw", Dest: "ips", Action: config.PolicyAccept}}
-		}},
+		}, "policy:0", 0, []string{"ips"}},
 		{"hosts-only zone", func(c *config.Config) {
 			c.Zones["hst"] = config.Zone{Type: config.ZoneIP}
 			c.Hosts = []config.Host{{Zone: "hst", Interface: "eth0", Addresses: []string{"192.0.2.0/24"}}}
 			c.Policy = []config.Policy{{Source: "fw", Dest: "hst", Action: config.PolicyAccept}}
-		}},
-		{"fw all expansion", func(c *config.Config) {
-			c.Zones["ips"] = config.Zone{Type: config.ZoneIPSec}
+		}, "policy:0", 0, []string{"hst"}},
+		{"fw all expansion keeps zones with interfaces", func(c *config.Config) {
+			ipsec(c)
 			c.Zones["hst"] = config.Zone{Type: config.ZoneIP}
 			c.Hosts = []config.Host{{Zone: "hst", Interface: "eth0", Addresses: []string{"192.0.2.0/24"}}}
 			c.Policy = []config.Policy{{Source: "fw", Dest: "all", Action: config.PolicyDrop}}
-		}},
+		}, "policy:0", 1, []string{"hst", "ips"}},
+		{"negated address does not scope", rule("ips:!192.0.2.1"), "rule:0", 0, []string{"ips"}},
+		{"address scopes", rule("ips:192.0.2.1"), "rule:0", 1, []string{"ips"}},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+			var logs bytes.Buffer
+			prev := slog.Default()
+			slog.SetDefault(slog.New(slog.NewTextHandler(&logs, nil)))
+			defer slog.SetDefault(prev)
+
 			state := mustCompile(t, listCfg(tt.mod))
+			got := 0
 			for chain := range state.Rules {
-				for _, r := range taggedRules(state, chain, "policy:0") {
+				for _, r := range taggedRules(state, chain, tt.tag) {
+					got++
 					if describeRule(r) == "" {
-						t.Errorf("%s: un-scoped policy rule", chain)
+						t.Errorf("%s: un-scoped rule", chain)
 					}
 				}
 			}
+			if got != tt.want {
+				t.Errorf("got %d %s rules, want %d", got, tt.tag, tt.want)
+			}
+			if n := strings.Count(logs.String(), "zone has no interfaces"); n != len(tt.warns) {
+				t.Errorf("got %d warnings, want %d:\n%s", n, len(tt.warns), logs.String())
+			}
+			for _, z := range tt.warns {
+				if n := strings.Count(logs.String(), "zone="+z+"\n"); n != 1 {
+					t.Errorf("zone %s warned %d times, want 1", z, n)
+				}
+			}
 		})
 	}
 }