Fall back to per-prefix filter rules when ipset is unavailable

client/firewall/iptables/family_linux.go

@@ -112,6 +112,10 @@ type family struct {
 	// AddFilterRule writes here; DeleteFilterRule looks up by id.
 	filters      map[nbid.RuleID]*Rule
 	ipsetCounter *ipsetCounter
+	// ipsetSupported records whether the kernel can create the hash:net
+	// sets the source matches rely on; probed once at init. When false,
+	// multi-source rules expand to one rule per source prefix.
+	ipsetSupported bool
 
 	// rules holds NAT, jump, and MSS-clamping rules (auxiliary
 	// plumbing that isn't a filter rule).
@@ -155,6 +159,8 @@ func newFamily(iptablesClient *iptables.IPTables, wgIface iFaceMapper, mtu uint1
 func (r *family) init(stateManager *statemanager.Manager) error {
 	r.stateManager = stateManager
 
+	r.ipsetSupported = r.probeIPSetSupport()
+
 	if err := r.cleanUpDefaultForwardRules(); err != nil {
 		log.Errorf("failed to clean up rules from FORWARD chain: %s", err)
 	}

client/firewall/iptables/filter_linux.go

@@ -38,14 +38,8 @@ func (r *family) AddFilterRule(
 		return existing, nil
 	}
 
-	srcMatch, err := r.applySourceMatch(sourceNetwork(sources), sources)
+	rule, err := r.installFilterRules(ruleID, sources, destination, proto, sPort, dPort, action, r.ipsetSupported)
 	if err != nil {
-		return nil, fmt.Errorf("apply source match: %w", err)
-	}
-
-	rule, err := r.installFilterRule(ruleID, srcMatch, destination, proto, sPort, dPort, action)
-	if err != nil {
-		r.dropSourceMatch(srcMatch)
 		return nil, err
 	}
 
@@ -54,6 +48,34 @@ func (r *family) AddFilterRule(
 	return rule, nil
 }
 
+// installFilterRules resolves the source matches and installs one
+// iptables rule per match. It is more than one rule only when useIPSet
+// is false and a multi-source rule has to be expanded per prefix.
+func (r *family) installFilterRules(
+	ruleID nbid.RuleID,
+	sources []netip.Prefix,
+	destination firewall.Network,
+	proto firewall.Protocol,
+	sPort *firewall.Port,
+	dPort *firewall.Port,
+	action firewall.Action,
+	useIPSet bool,
+) (*Rule, error) {
+	srcMatches, err := r.applySourceMatches(sources, useIPSet)
+	if err != nil {
+		return nil, fmt.Errorf("apply source match: %w", err)
+	}
+
+	rule, err := r.installFilterRule(ruleID, srcMatches, destination, proto, sPort, dPort, action)
+	if err != nil {
+		for _, srcMatch := range srcMatches {
+			r.dropSourceMatch(srcMatch)
+		}
+		return nil, err
+	}
+	return rule, nil
+}
+
 func (r *family) hasRule(id nbid.RuleID) bool {
 	_, ok := r.filters[id]
 	return ok
@@ -80,25 +102,28 @@ func (r *family) DeleteFilterRule(rule firewall.Rule) error {
 		return nil
 	}
 
-	// DeleteIfExists keeps both deletes idempotent so a retry after a
-	// partial failure does not error on the half that was already removed.
+	// DeleteIfExists keeps the deletes idempotent so a retry after a
+	// partial failure does not error on the parts already removed.
 	var merr *multierror.Error
-	if err := r.iptablesClient.DeleteIfExists(tableFilter, pr.chain, pr.specs...); err != nil {
-		merr = multierror.Append(merr, fmt.Errorf("delete rule from %s: %w", pr.chain, err))
-	}
-	if pr.mangleSpecs != nil {
-		if err := r.iptablesClient.DeleteIfExists(tableMangle, chainRTPre, pr.mangleSpecs...); err != nil {
-			merr = multierror.Append(merr, fmt.Errorf("delete mangle rule: %w", err))
+	for _, fs := range pr.allSpecs() {
+		if err := r.iptablesClient.DeleteIfExists(tableFilter, pr.chain, fs.specs...); err != nil {
+			merr = multierror.Append(merr, fmt.Errorf("delete rule from %s: %w", pr.chain, err))
+		}
+		if fs.mangleSpecs != nil {
+			if err := r.iptablesClient.DeleteIfExists(tableMangle, chainRTPre, fs.mangleSpecs...); err != nil {
+				merr = multierror.Append(merr, fmt.Errorf("delete mangle rule: %w", err))
+			}
 		}
 	}
 	if merr != nil {
-		// Leave the rule tracked so the caller retries the remaining half.
+		// Leave the rule tracked so the caller retries the remaining part.
 		return nberrors.FormatErrorOrNil(merr)
 	}
 
 	// The rule is gone from iptables, so untrack it regardless of how the
 	// refcount decrement goes, but surface decrement failures so callers
-	// see the ipset desync.
+	// see the ipset desync. Only the primary spec can reference sets: the
+	// per-prefix expansion never uses them.
 	delete(r.filters, ruleID)
 	r.updateState()
 	if err := r.decrementSetCounter(pr.specs); err != nil {
@@ -136,6 +161,28 @@ func sourceNetwork(sources []netip.Prefix) firewall.Network {
 	}
 }
 
+// applySourceMatches returns one source match fragment per iptables
+// rule needed for the sources: normally a single fragment (a set match,
+// a direct -s match, or nil for match-any), and one -s fragment per
+// prefix when a multi-source rule cannot use ipset. Per-prefix rules
+// are the only form a kernel without the ipset modules can express.
+func (r *family) applySourceMatches(sources []netip.Prefix, useIPSet bool) ([][]string, error) {
+	network := sourceNetwork(sources)
+	if !network.IsSet() || useIPSet {
+		match, err := r.applySourceMatch(network, sources)
+		if err != nil {
+			return nil, err
+		}
+		return [][]string{match}, nil
+	}
+
+	matches := make([][]string, 0, len(sources))
+	for _, source := range sources {
+		matches = append(matches, []string{"-s", source.String()})
+	}
+	return matches, nil
+}
+
 // applySourceMatch returns the iptables match fragment for the rule's
 // source. For a Set it increments the shared ipset's refcount; for a
 // Prefix it emits a direct -s match; for the wildcard it returns nil.
@@ -188,14 +235,15 @@ func (r *family) decrementSetCounter(rule []string) error {
 	return nberrors.FormatErrorOrNil(merr)
 }
 
-// installFilterRule assembles and writes one iptables filter-chain
-// rule. With destination empty the rule lands in the peer ACL input
-// chain and a paired mangle PREROUTING rule is added for the redirect
-// mark. With destination set the rule lands in the route ACL forward
-// chain and there is no mangle pairing.
+// installFilterRule assembles and writes the iptables filter-chain
+// rules for one filter rule, one per source match fragment. With
+// destination empty the rules land in the peer ACL input chain and each
+// gets a paired mangle PREROUTING rule for the redirect mark. With
+// destination set the rules land in the route ACL forward chain and
+// there is no mangle pairing.
 func (r *family) installFilterRule(
 	ruleID nbid.RuleID,
-	srcMatch []string,
+	srcMatches [][]string,
 	destination firewall.Network,
 	protocol firewall.Protocol,
 	sPort, dPort *firewall.Port,
@@ -205,7 +253,6 @@ func (r *family) installFilterRule(
 
 	proto := protoForFamily(protocol, r.v6)
 
-	specs := slices.Clone(srcMatch)
 	var destExp []string
 	if isRoute {
 		var err error
@@ -213,64 +260,93 @@ func (r *family) installFilterRule(
 		if err != nil {
 			return nil, fmt.Errorf("apply network -d: %w", err)
 		}
-		specs = append(specs, destExp...)
 	}
-	specs = append(specs, filterMatchSpecs(proto, sPort, dPort)...)
-
-	var mangleSpecs []string
-	if !isRoute {
-		mangleSpecs = slices.Clone(specs)
-		mangleSpecs = append(mangleSpecs,
-			"-i", r.wgIface.Name(),
-			"-m", "addrtype", "--dst-type", "LOCAL",
-			"-j", "MARK", "--set-xmark", fmt.Sprintf("%#x", nbnet.PreroutingFwmarkRedirected),
-		)
-	}
-
-	specs = append(specs, "-j", actionToStr(action))
+	matchSpecs := filterMatchSpecs(proto, sPort, dPort)
 
 	chain := chainACLInput
 	if isRoute {
 		chain = chainRTFwdIn
 	}
 
-	// Peer ACL drops are inserted at position 1 so they precede the
-	// chain's catch-all; route ACL drops are inserted at position 2
-	// to sit immediately after the established/related accept rule.
-	var err error
-	if action == firewall.ActionDrop {
-		pos := 1
-		if isRoute {
-			pos = 2
-		}
-		err = r.iptablesClient.Insert(tableFilter, chain, pos, specs...)
-	} else {
-		err = r.iptablesClient.Append(tableFilter, chain, specs...)
-	}
-	if err != nil {
-		r.dropSourceMatch(destExp)
-		return nil, fmt.Errorf("install filter rule on %s: %w", chain, err)
-	}
+	var installed []filterSpecs
+	for _, srcMatch := range srcMatches {
+		specs := slices.Clone(srcMatch)
+		specs = append(specs, destExp...)
+		specs = append(specs, matchSpecs...)
 
-	// The mangle redirect-mark rule is best effort: the filter rule itself
-	// is what enforces the ACL, so a mangle failure must not undo it. Drop
-	// the spec so teardown does not try to remove a rule that was not added.
-	if mangleSpecs != nil {
-		if err := r.iptablesClient.Append(tableMangle, chainRTPre, mangleSpecs...); err != nil {
-			log.Errorf("add mangle rule: %v", err)
-			mangleSpecs = nil
+		var mangleSpecs []string
+		if !isRoute {
+			mangleSpecs = slices.Clone(specs)
+			mangleSpecs = append(mangleSpecs,
+				"-i", r.wgIface.Name(),
+				"-m", "addrtype", "--dst-type", "LOCAL",
+				"-j", "MARK", "--set-xmark", fmt.Sprintf("%#x", nbnet.PreroutingFwmarkRedirected),
+			)
 		}
+
+		specs = append(specs, "-j", actionToStr(action))
+
+		if err := r.insertFilterRule(chain, action, specs); err != nil {
+			// Leave nothing half-installed: the caller sees an error, so a
+			// partial rule would silently keep matching without being tracked.
+			r.removeFilterSpecs(chain, installed)
+			r.dropSourceMatch(destExp)
+			return nil, fmt.Errorf("install filter rule on %s: %w", chain, err)
+		}
+
+		// The mangle redirect-mark rule is best effort: the filter rule itself
+		// is what enforces the ACL, so a mangle failure must not undo it. Drop
+		// the spec so teardown does not try to remove a rule that was not added.
+		if mangleSpecs != nil {
+			if err := r.iptablesClient.Append(tableMangle, chainRTPre, mangleSpecs...); err != nil {
+				log.Errorf("add mangle rule: %v", err)
+				mangleSpecs = nil
+			}
+		}
+
+		installed = append(installed, filterSpecs{specs: specs, mangleSpecs: mangleSpecs})
 	}
 
 	return &Rule{
 		id:          ruleID,
-		specs:       specs,
-		mangleSpecs: mangleSpecs,
+		specs:       installed[0].specs,
+		mangleSpecs: installed[0].mangleSpecs,
+		extraRules:  installed[1:],
 		chain:       chain,
 		v6:          r.v6,
 	}, nil
 }
 
+// insertFilterRule writes one assembled rule spec into the given ACL
+// chain. Peer ACL drops are inserted at position 1 so they precede the
+// chain's catch-all; route ACL drops are inserted at position 2 to sit
+// immediately after the established/related accept rule.
+func (r *family) insertFilterRule(chain string, action firewall.Action, specs []string) error {
+	if action == firewall.ActionDrop {
+		pos := 1
+		if chain == chainRTFwdIn {
+			pos = 2
+		}
+		return r.iptablesClient.Insert(tableFilter, chain, pos, specs...)
+	}
+	return r.iptablesClient.Append(tableFilter, chain, specs...)
+}
+
+// removeFilterSpecs deletes the already-installed rules of a partially
+// applied filter rule.
+func (r *family) removeFilterSpecs(chain string, installed []filterSpecs) {
+	for _, fs := range installed {
+		if err := r.iptablesClient.DeleteIfExists(tableFilter, chain, fs.specs...); err != nil {
+			log.Debugf("delete partial filter rule: %v", err)
+		}
+		if fs.mangleSpecs != nil {
+			if err := r.iptablesClient.DeleteIfExists(tableMangle, chainRTPre, fs.mangleSpecs...); err != nil {
+				log.Debugf("delete partial mangle rule: %v", err)
+			}
+		}
+	}
+}
+
 // applyNetwork resolves a firewall.Network into the iptables match
 // fragment for the given direction flag (-s or -d). Set networks
 // increment the shared ipset refcount; prefixes emit a direct match;

client/firewall/iptables/ipset_linux.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"net/netip"
 
+	"github.com/google/uuid"
 	"github.com/hashicorp/go-multierror"
 	"github.com/lrh3321/ipset-go"
 	log "github.com/sirupsen/logrus"
@@ -14,6 +15,32 @@ import (
 	firewall "github.com/netbirdio/netbird/client/firewall/manager"
 )
 
+// probeIPSetSupport checks whether the kernel can create the ipset type
+// used for source and destination matches. On kernels lacking the
+// required ipset hash module, set creation fails (e.g. "invalid
+// argument"), which would otherwise fail every multi-source rule and
+// leave traffic the policy permits blocked by the catch-all drop. When
+// unsupported, multi-source rules fall back to one rule per prefix.
+func (r *family) probeIPSetSupport() bool {
+	// Use a unique name so concurrent processes don't collide and we only ever
+	// destroy the set we created ourselves. ipset names are limited to 31 chars,
+	// so use a short random suffix.
+	probeName := "nb-probe-" + uuid.New().String()[:8]
+
+	if err := r.createIPSet(probeName); err != nil {
+		log.Warnf("ipset is not available (failed to create probe set: %v); "+
+			"falling back to per-IP iptables ACL rules. Ensure the kernel provides "+
+			"the ipset hash:net module (ip_set_hash_net) for better performance with large rule sets", err)
+		return false
+	}
+
+	if err := r.destroyIPSet(probeName); err != nil {
+		log.Debugf("destroy ipset probe set %q: %v", probeName, err)
+	}
+
+	return true
+}
+
 func (r *family) createIpSet(setName string, sources []netip.Prefix) error {
 	if err := r.createIPSet(setName); err != nil {
 		return fmt.Errorf("create set %s: %w", setName, err)

client/firewall/iptables/manager_linux_test.go

@@ -247,6 +247,99 @@ func TestIptablesManagerIPSet(t *testing.T) {
 	})
 }
 
+// TestIptablesFilterIPSetFallback verifies that when the kernel lacks
+// ipset support, a multi-source rule falls back to one iptables rule
+// per source prefix instead of silently leaving the chain empty. See
+// discussion #6125.
+func TestIptablesFilterIPSetFallback(t *testing.T) {
+	ipv4Client, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
+	require.NoError(t, err)
+
+	manager, err := Create(ifaceMock, iface.DefaultMTU)
+	require.NoError(t, err)
+	require.NoError(t, manager.Init(nil))
+
+	defer func() {
+		require.NoError(t, manager.Close(nil))
+	}()
+
+	// Simulate a kernel without the ipset hash module.
+	manager.family4.ipsetSupported = false
+
+	sources := []netip.Prefix{
+		netip.MustParsePrefix("10.20.0.42/32"),
+		netip.MustParsePrefix("10.20.0.43/32"),
+	}
+	port := &fw.Port{Values: []uint16{22}}
+
+	rule, err := manager.AddFilterRule(nil, sources, fw.Network{}, "tcp", nil, port, fw.ActionAccept)
+	require.NoError(t, err, "AddFilterRule should succeed via fallback")
+
+	rr := rule.(*Rule)
+	all := rr.allSpecs()
+	require.Len(t, all, len(sources), "each source prefix needs its own rule")
+	for i, fs := range all {
+		joined := strings.Join(fs.specs, " ")
+		require.Contains(t, joined, "-s "+sources[i].String(), "fallback rule must match by source prefix")
+		require.NotContains(t, joined, matchSet, "fallback rule must not use ipset matching")
+
+		// The rule must actually be present in the ACL chain (not silently dropped).
+		checkRuleSpecs(t, ipv4Client, rr.chain, true, fs.specs...)
+	}
+
+	require.NoError(t, manager.DeleteFilterRule(rule), "failed to delete fallback rule")
+	for _, fs := range all {
+		checkRuleSpecs(t, ipv4Client, rr.chain, false, fs.specs...)
+	}
+}
+
+// TestIptablesRouteFilterIPSetFallback covers the route ACL side of the
+// fallback: with a destination set, the expanded per-source rules land
+// in the route forward chain and are all removed on delete.
+func TestIptablesRouteFilterIPSetFallback(t *testing.T) {
+	ipv4Client, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
+	require.NoError(t, err)
+
+	manager, err := Create(ifaceMock, iface.DefaultMTU)
+	require.NoError(t, err)
+	require.NoError(t, manager.Init(nil))
+
+	defer func() {
+		require.NoError(t, manager.Close(nil))
+	}()
+
+	manager.family4.ipsetSupported = false
+
+	sources := []netip.Prefix{
+		netip.MustParsePrefix("172.16.0.0/16"),
+		netip.MustParsePrefix("192.168.0.0/16"),
+	}
+	destination := fw.Network{Prefix: netip.MustParsePrefix("10.0.0.0/8")}
+	port := &fw.Port{Values: []uint16{443}}
+
+	rule, err := manager.AddFilterRule(nil, sources, destination, "tcp", nil, port, fw.ActionAccept)
+	require.NoError(t, err, "route ACL must install without ipset")
+
+	rr := rule.(*Rule)
+	require.Equal(t, chainRTFwdIn, rr.chain, "route rule must land in the forward chain")
+
+	all := rr.allSpecs()
+	require.Len(t, all, len(sources), "each source prefix needs its own rule")
+	for i, fs := range all {
+		joined := strings.Join(fs.specs, " ")
+		require.Contains(t, joined, "-s "+sources[i].String(), "fallback rule must match by source prefix")
+		require.NotContains(t, joined, matchSet, "fallback rule must not use ipset matching")
+		require.Nil(t, fs.mangleSpecs, "route rules have no mangle pairing")
+
+		checkRuleSpecs(t, ipv4Client, rr.chain, true, fs.specs...)
+	}
+
+	require.NoError(t, manager.DeleteFilterRule(rule), "failed to delete fallback rule")
+	for _, fs := range all {
+		checkRuleSpecs(t, ipv4Client, rr.chain, false, fs.specs...)
+	}
+}
+
 func checkRuleSpecs(t *testing.T, ipv4Client *iptables.IPTables, chainName string, mustExists bool, rulespec ...string) {
 	t.Helper()
 	exists, err := ipv4Client.Exists("filter", chainName, rulespec...)

client/firewall/iptables/rule.go

@@ -10,8 +10,25 @@ type Rule struct {
 	id          manager.RuleID
 	specs       []string
 	mangleSpecs []string
-	chain       string
-	v6          bool
+	// extraRules holds the rules beyond the first when the ipset
+	// fallback expands a multi-source rule into one rule per prefix.
+	extraRules []filterSpecs
+	chain      string
+	v6         bool
+}
+
+// filterSpecs is one installed iptables rule: its filter-table spec and
+// the paired mangle redirect-mark spec (nil for route rules or when the
+// mangle rule could not be added).
+type filterSpecs struct {
+	specs       []string
+	mangleSpecs []string
+}
+
+// allSpecs returns the spec pairs of every iptables rule backing this
+// Rule, the primary one first.
+func (r *Rule) allSpecs() []filterSpecs {
+	return append([]filterSpecs{{specs: r.specs, mangleSpecs: r.mangleSpecs}}, r.extraRules...)
 }
 
 // ID returns the rule id