Merge pull request 'Exclude sub-zone hosts from wildcard parent interfaces' (#39) from benvin/wildcard-subzone-exclusion into main

Reviewed-on: #39

internal/config/config_test.go

@@ -473,6 +473,20 @@ func TestValidateHosts(t *testing.T) {
 			},
 			wantErr: "interface \"eth99\" not defined in interfaces",
 		},
+		{
+			name: "host interface matched by wildcard",
+			zones: map[string]Zone{
+				"fw":  {Type: ZoneFirewall},
+				"net": {Type: ZoneIP},
+				"lan": {Type: ZoneIP, Parents: []string{"net"}},
+			},
+			interfaces: []Interface{
+				{Zone: "net", Interface: "enp+"},
+			},
+			hosts: []Host{
+				{Zone: "lan", Interface: "enp2s0", Addresses: []string{"192.0.2.0/24"}},
+			},
+		},
 		{
 			name: "zone not defined",
 			zones: map[string]Zone{

internal/config/hosts.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"net/netip"
 	"slices"
+	"strings"
 )
 
 type Host struct {
@@ -44,7 +45,8 @@ func (c *Config) validateHosts() error {
 
 		ifaceFound := false
 		for _, iface := range c.Interfaces {
-			if iface.Interface == h.Interface || iface.PhysicalName() == h.Interface {
+			prefix, wild := strings.CutSuffix(iface.PhysicalName(), "+")
+			if iface.Interface == h.Interface || iface.PhysicalName() == h.Interface || (wild && strings.HasPrefix(h.Interface, prefix)) {
 				ifaceFound = true
 				break
 			}

internal/nftables/compiler.go

@@ -1278,9 +1278,11 @@ func (c *Compiler) selectChain(srcZone, dstZone, fwZone string) string {
 // zoneMatch classifies a packet into a zone: an interface (empty: any) and, for a hosts entry, one host
 // address; excl carves out hosts exclusions and the hosts of sub-zones, which shorewall matches first.
 // fam (an NFPROTO; addr's family when set, else 0: any) guards addr and excl so IPv4 offsets are
-// never compared against IPv6 bytes. routeback marks a hosts entry with the routeback option.
+// never compared against IPv6 bytes. routeback marks a hosts entry with the routeback option. notIface
+// carves narrower sub-zone host interfaces out of a wildcard iface; they get entries of their own.
 type zoneMatch struct {
 	iface, addr string
+	notIface    []string
 	excl        []string
 	fam         byte
 	routeback   bool
@@ -1304,17 +1306,18 @@ func (c *Compiler) resolveZone(zone, addr string) []zoneMatch {
 	var out []zoneMatch
 	hasHosts := false
 	for _, iface := range c.cfg.ZoneInterfaces(zone) {
-		sub, back := c.subZoneHosts(zone, iface)
+		for _, sp := range c.subZoneSplit(zone, iface) {
-		if len(sub) == 0 {
+			if len(sp.sub) == 0 {
-			out = append(out, zoneMatch{iface: iface})
+				out = append(out, zoneMatch{iface: sp.iface, notIface: sp.not})
-		} else {
+			} else {
-			v4, v6 := splitFamily(sub)
+				v4, v6 := splitFamily(sp.sub)
-			out = append(out, zoneMatch{iface: iface, excl: v4, fam: unix.NFPROTO_IPV4},
+				out = append(out, zoneMatch{iface: sp.iface, notIface: sp.not, excl: v4, fam: unix.NFPROTO_IPV4},
-				zoneMatch{iface: iface, excl: v6, fam: unix.NFPROTO_IPV6})
+					zoneMatch{iface: sp.iface, notIface: sp.not, excl: v6, fam: unix.NFPROTO_IPV6})
-		}
+			}
-		for _, b := range back {
+			for _, b := range sp.back {
-			if addrsOverlap(b, addr) {
+				if addrsOverlap(b, addr) {
-				out = append(out, zoneMatch{iface: iface, addr: b})
+					out = append(out, zoneMatch{iface: sp.iface, notIface: sp.not, addr: b})
+				}
 			}
 		}
 	}
@@ -1323,15 +1326,16 @@ func (c *Compiler) resolveZone(zone, addr string) []zoneMatch {
 			continue
 		}
 		hasHosts = true
-		sub, _ := c.subZoneHosts(zone, h.Interface)
+		for _, sp := range c.subZoneSplit(zone, h.Interface) {
-		v4, v6 := splitFamily(slices.Concat(h.Exclusions, sub))
+			v4, v6 := splitFamily(slices.Concat(h.Exclusions, sp.sub))
-		for _, a := range h.Addresses {
+			for _, a := range h.Addresses {
-			if addrsOverlap(a, addr) {
+				if addrsOverlap(a, addr) {
-				m := zoneMatch{iface: h.Interface, addr: a, excl: v6, routeback: h.Options.RouteBack}
+					m := zoneMatch{iface: sp.iface, notIface: sp.not, addr: a, excl: v6, routeback: h.Options.RouteBack}
-				if p, err := parsePrefix(a); err == nil && p.Addr().Is4() {
+					if p, err := parsePrefix(a); err == nil && p.Addr().Is4() {
-					m.excl = v4
+						m.excl = v4
+					}
+					out = append(out, m)
 				}
-				out = append(out, m)
 			}
 		}
 	}
@@ -1351,16 +1355,46 @@ func (c *Compiler) resolveZone(zone, addr string) []zoneMatch {
 	return nil
 }
 
-// subZoneHosts lists the host addresses on iface that belong to sub-zones of zone, and those
+// subZoneSplit partitions iface for zone's sub-zone hosts, as shorewall matches them: the hosts on an
-// sub-zone hosts' exclusions, which fall back to zone.
+// interface covering iface exclude their addresses (sub) on all of it, while a host interface strictly
-func (c *Compiler) subZoneHosts(zone, iface string) (sub, back []string) {
+// inside a wildcard iface is carved out (not) into its own entry, so its addresses stay in zone on every
+// other interface the wildcard matches. back lists the sub-zone hosts' exclusions, which fall back to zone.
+type subZoneSplit struct {
+	iface     string
+	not       []string
+	sub, back []string
+}
+
+func (c *Compiler) subZoneSplit(zone, iface string) []subZoneSplit {
+	top := subZoneSplit{iface: iface}
+	var inner []string
 	for _, h := range c.cfg.Hosts {
-		if h.Interface == iface && c.cfg.IsSubZone(h.Zone, zone) {
+		switch {
-			sub = append(sub, h.Addresses...)
+		case !c.cfg.IsSubZone(h.Zone, zone):
-			back = append(back, h.Exclusions...)
+		case ifaceCovers(h.Interface, iface):
+			top.sub = append(top.sub, h.Addresses...)
+			top.back = append(top.back, h.Exclusions...)
+		case ifaceCovers(iface, h.Interface) && !slices.Contains(inner, h.Interface):
+			inner = append(inner, h.Interface)
 		}
 	}
-	return sub, back
+	var rest []subZoneSplit
+	for _, h := range inner {
+		if slices.ContainsFunc(inner, func(o string) bool { return o != h && ifaceCovers(o, h) }) {
+			continue
+		}
+		top.not = append(top.not, h)
+		rest = append(rest, c.subZoneSplit(zone, h)...)
+	}
+	return append([]subZoneSplit{top}, rest...)
+}
+
+// ifaceCovers reports whether every interface name b matches is also matched by a ("+" suffix: prefix wildcard).
+func ifaceCovers(a, b string) bool {
+	if pa, ok := strings.CutSuffix(a, "+"); ok {
+		return strings.HasPrefix(strings.TrimSuffix(b, "+"), pa)
+	}
+	return a == b
 }
 
 // addrsOverlap reports whether a host address can match a rule address; unparsable or negated rule addresses keep the host.
@@ -1406,6 +1440,11 @@ func zoneMatchExprs(m zoneMatch, src bool) ([]expr.Any, error) {
 	if m.iface != "" {
 		out = matchIfaceName(src, m.iface)
 	}
+	for _, n := range m.notIface {
+		e := matchIfaceName(src, n)
+		e[1].(*expr.Cmp).Op = expr.CmpOpNeq
+		out = append(out, e...)
+	}
 	var addr []expr.Any
 	if m.addr != "" {
 		p, err := parsePrefix(m.addr)

internal/nftables/compiler_test.go

@@ -1921,9 +1921,17 @@ func describeRule(r ManagedRule) string {
 		case *expr.Meta:
 			switch m.Key {
 			case expr.MetaKeyIIFNAME:
-				parts = append(parts, "iif="+strings.TrimRight(string(cmp.Data), "\x00"))
+				op := "="
+				if cmp.Op == expr.CmpOpNeq {
+					op = "!="
+				}
+				parts = append(parts, "iif"+op+strings.TrimRight(string(cmp.Data), "\x00"))
 			case expr.MetaKeyOIFNAME:
-				parts = append(parts, "oif="+strings.TrimRight(string(cmp.Data), "\x00"))
+				op := "="
+				if cmp.Op == expr.CmpOpNeq {
+					op = "!="
+				}
+				parts = append(parts, "oif"+op+strings.TrimRight(string(cmp.Data), "\x00"))
 			case expr.MetaKeyNFPROTO:
 				parts = append(parts, map[byte]string{unix.NFPROTO_IPV4: "ip4", unix.NFPROTO_IPV6: "ip6"}[cmp.Data[0]])
 			}
@@ -3663,3 +3671,70 @@ func TestCompile_HostsRouteBack(t *testing.T) {
 		t.Errorf("routeback hosts lan lan policy = %q, want %q", got, want)
 	}
 }
+
+func TestCompile_WildcardParentExcludesSubZoneHosts(t *testing.T) {
+	cfg := &config.Config{
+		Settings: config.Settings{TableName: "test", AddressFamily: config.FamilyINET},
+		Zones: map[string]config.Zone{
+			"fw": {Type: config.ZoneFirewall}, "net": {Type: config.ZoneIP},
+			"lan": {Type: config.ZoneIP, Parents: []string{"net"}}, "lxd": {Type: config.ZoneIP},
+		},
+		Interfaces: []config.Interface{
+			{Zone: "net", Interface: "enp+"}, {Zone: "net", Interface: "wlo1"}, {Zone: "lxd", Interface: "lxdbr0"},
+		},
+		Hosts: []config.Host{
+			{Zone: "lan", Interface: "enp2s0", Addresses: []string{"192.0.2.0/24"}},
+			{Zone: "lan", Interface: "enp4+", Addresses: []string{"203.0.113.0/24"}},
+			{Zone: "lan", Interface: "wlo1", Addresses: []string{"198.51.100.0/24"}},
+		},
+		Policy: []config.Policy{
+			{Source: "lxd", Dest: "net", Action: config.PolicyAccept},
+			{Source: "fw", Dest: "net", Action: config.PolicyDrop},
+			{Source: "net", Dest: "all", Action: config.PolicyDrop},
+			{Source: "all", Dest: "all", Action: config.PolicyReject},
+		},
+		PortGroups: make(map[string]config.PortGroup),
+	}
+	state := mustCompile(t, cfg)
+	for _, tt := range []struct{ chain, tag, dir string }{
+		{"forward", "policy:0", "iif=lxdbr0 oif"}, {"output", "policy:1", "oif"}, {"input", "policy:2", "iif"},
+	} {
+		d, a := tt.dir, "daddr"
+		if tt.chain == "input" {
+			a = "saddr"
+		}
+		want := []string{
+			d + "=enp " + d[len(d)-3:] + "!=enp2s0 " + d[len(d)-3:] + "!=enp4",
+			d + "=enp2s0 ip4 !" + a + "=192.0.2.0/24", d + "=enp2s0 ip6",
+			d + "=enp4 ip4 !" + a + "=203.0.113.0/24", d + "=enp4 ip6",
+			d + "=wlo1 ip4 !" + a + "=198.51.100.0/24", d + "=wlo1 ip6",
+		}
+		if got := describeTagged(state, tt.chain, tt.tag); !reflect.DeepEqual(got, want) {
+			t.Errorf("%s %s = %q, want %q", tt.chain, tt.tag, got, want)
+		}
+	}
+	for chain, want := range map[string]string{
+		"forward": "iif=lxdbr0 oif=enp2s0 ip4 daddr=192.0.2.0/24",
+		"output":  "oif=enp2s0 ip4 daddr=192.0.2.0/24",
+		"input":   "iif=enp2s0 ip4 saddr=192.0.2.0/24",
+	} {
+		if got := describeTagged(state, chain, "policy:3"); !slices.Contains(got, want) {
+			t.Errorf("%s policy:3 (lan reject) = %q, want it to contain %q", chain, got, want)
+		}
+	}
+}
+
+func TestIfaceCovers(t *testing.T) {
+	for _, tt := range []struct {
+		a, b string
+		want bool
+	}{
+		{"enp2s0", "enp2s0", true}, {"enp2s0", "enp3s0", false},
+		{"enp+", "enp2s0", true}, {"enp2s0", "enp+", false}, {"enp+", "wlo1", false},
+		{"en+", "enp+", true}, {"enp+", "en+", false}, {"enp+", "eno+", false}, {"enp+", "enp", true},
+	} {
+		if got := ifaceCovers(tt.a, tt.b); got != tt.want {
+			t.Errorf("ifaceCovers(%q, %q) = %v, want %v", tt.a, tt.b, got, tt.want)
+		}
+	}
+}