Preserve pre-existing ipsets and roll back partial route ACL installs

client/firewall/iptables/acl_linux.go

@@ -42,12 +42,12 @@ type aclManager struct {
 	optionalEntries map[string][]entry
 	ipsetStore      *ipsetStore
 	v6              bool
-	ipsetSupported  bool
+	ipsetSupport    *ipsetSupport
 
 	stateManager *statemanager.Manager
 }
 
-func newAclManager(iptablesClient *iptables.IPTables, wgIface iFaceMapper) (*aclManager, error) {
+func newAclManager(iptablesClient *iptables.IPTables, wgIface iFaceMapper, ipsetSupport *ipsetSupport) (*aclManager, error) {
 	return &aclManager{
 		iptablesClient:  iptablesClient,
 		wgIface:         wgIface,
@@ -55,14 +55,13 @@ func newAclManager(iptablesClient *iptables.IPTables, wgIface iFaceMapper) (*acl
 		optionalEntries: make(map[string][]entry),
 		ipsetStore:      newIpsetStore(),
 		v6:              iptablesClient.Proto() == iptables.ProtocolIPv6,
+		ipsetSupport:    ipsetSupport,
 	}, nil
 }
 
 func (m *aclManager) init(stateManager *statemanager.Manager) error {
 	m.stateManager = stateManager
 
-	m.ipsetSupported = m.probeIPSetSupport()
-
 	m.seedInitialEntries()
 	m.seedInitialOptionalEntries()
 
@@ -88,18 +87,77 @@ func (m *aclManager) AddPeerFiltering(
 	action firewall.Action,
 	ipsetName string,
 ) ([]firewall.Rule, error) {
-	chain := chainNameInputRules
+	ipsetName = m.resolveIPSetName(ipsetName, sPort, dPort, action)
+	if ipsetName == "" {
+		return m.addPeerRule(ip, protocol, sPort, dPort, action, "")
+	}
 
+	// A set that is already in the store backs rules installed earlier, so it must
+	// survive this call's failure.
+	_, preexisting := m.ipsetStore.ipset(ipsetName)
+
+	rules, err := m.addPeerRule(ip, protocol, sPort, dPort, action, ipsetName)
+	if err == nil {
+		return rules, nil
+	}
+
+	var unusable *ipsetUnusableError
+	if !errors.As(err, &unusable) {
+		return nil, err
+	}
+
+	// The set could not be created or matched. Drop the one this call created and
+	// retry the rule matching the IP directly; only if that succeeds do we know
+	// ipset was to blame and latch it off for subsequent rules.
+	if !preexisting {
+		m.discardIPSet(ipsetName)
+	}
+
+	rules, retryErr := m.addPeerRule(ip, protocol, sPort, dPort, action, "")
+	if retryErr != nil {
+		return nil, fmt.Errorf("add peer rule (ipset: %w): %w", unusable.cause, retryErr)
+	}
+
+	m.ipsetSupport.markUnsupported(unusable.cause)
+
+	return rules, nil
+}
+
+// resolveIPSetName derives the ipset name for a rule, returning "" when the rule
+// must match the IP directly: either no set was requested or ipset is unusable.
+func (m *aclManager) resolveIPSetName(ipsetName string, sPort, dPort *firewall.Port, action firewall.Action) string {
 	ipsetName = transformIPsetName(ipsetName, sPort, dPort, action)
-	if m.v6 && ipsetName != "" {
+	if ipsetName == "" || !m.ipsetSupport.supported() {
+		return ""
+	}
+
+	if m.v6 {
 		ipsetName += "-v6"
 	}
-	// When the kernel lacks the required ipset hash module, fall back to
-	// per-IP iptables rules (pre-0.68 behavior) so ACLs keep working instead
-	// of silently leaving the chain empty.
-	if ipsetName != "" && !m.ipsetSupported {
-		ipsetName = ""
+
+	return ipsetName
+}
+
+// discardIPSet removes a set that turned out to be unusable, so a later rule
+// does not find it in the store and assume it works.
+func (m *aclManager) discardIPSet(ipsetName string) {
+	m.ipsetStore.deleteIpset(ipsetName)
+
+	if err := m.destroyIPSet(ipsetName); err != nil {
+		log.Debugf("destroy unusable ipset %s: %v", ipsetName, err)
 	}
+}
+
+func (m *aclManager) addPeerRule(
+	ip net.IP,
+	protocol firewall.Protocol,
+	sPort *firewall.Port,
+	dPort *firewall.Port,
+	action firewall.Action,
+	ipsetName string,
+) ([]firewall.Rule, error) {
+	chain := chainNameInputRules
+
 	proto := protoForFamily(protocol, m.v6)
 	specs := filterRuleSpecs(ip, proto, sPort, dPort, action, ipsetName)
 
@@ -114,7 +172,7 @@ func (m *aclManager) AddPeerFiltering(
 	if ipsetName != "" {
 		if ipList, ipsetExists := m.ipsetStore.ipset(ipsetName); ipsetExists {
 			if err := m.addToIPSet(ipsetName, ip); err != nil {
-				return nil, fmt.Errorf("add IP to ipset: %w", err)
+				return nil, ipsetUnusable(fmt.Errorf("add IP to ipset: %w", err))
 			}
 			// if ruleset already exists it means we already have the firewall rule
 			// so we need to update IPs in the ruleset and return new fw.Rule object for ACL manager.
@@ -137,10 +195,10 @@ func (m *aclManager) AddPeerFiltering(
 			}
 		}
 		if err := m.createIPSet(ipsetName); err != nil {
-			return nil, fmt.Errorf("create ipset: %w", err)
+			return nil, ipsetUnusable(fmt.Errorf("create ipset: %w", err))
 		}
 		if err := m.addToIPSet(ipsetName, ip); err != nil {
-			return nil, fmt.Errorf("add IP to ipset: %w", err)
+			return nil, ipsetUnusable(fmt.Errorf("add IP to ipset: %w", err))
 		}
 
 		ipList := newIpList(ip.String())
@@ -149,7 +207,7 @@ func (m *aclManager) AddPeerFiltering(
 
 	ok, err := m.iptablesClient.Exists(tableFilter, chain, specs...)
 	if err != nil {
-		return nil, fmt.Errorf("failed to check rule: %w", err)
+		return nil, maybeIPSetUnusable(ipsetName, fmt.Errorf("check rule: %w", err))
 	}
 	if ok {
 		return nil, fmt.Errorf("rule already exists")
@@ -163,7 +221,7 @@ func (m *aclManager) AddPeerFiltering(
 		err = m.iptablesClient.Append(tableFilter, chain, specs...)
 	}
 	if err != nil {
-		return nil, err
+		return nil, maybeIPSetUnusable(ipsetName, err)
 	}
 
 	if err := m.iptablesClient.Append(tableMangle, chainRTPRE, mangleSpecs...); err != nil {
@@ -507,40 +565,6 @@ func transformIPsetName(ipsetName string, sPort, dPort *firewall.Port, action fi
 	}
 }
 
-// probeIPSetSupport checks whether the kernel can create the ipset type used for
-// ACL rules. On kernels lacking the required ipset hash module, ipset creation
-// fails (e.g. "invalid argument"), which would otherwise leave the ACL chain
-// empty and silently drop all policy-permitted inbound traffic. When unsupported,
-// the manager falls back to per-IP iptables rules.
-func (m *aclManager) 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]
-
-	opts := ipset.CreateOptions{
-		Replace: true,
-	}
-	if m.v6 {
-		opts.Family = ipset.FamilyIPV6
-	}
-
-	if err := ipset.Create(probeName, ipset.TypeHashNet, opts); 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
-	}
-
-	defer func() {
-		if err := ipset.Destroy(probeName); err != nil {
-			log.Debugf("destroy ipset probe set %q: %v", probeName, err)
-		}
-	}()
-
-	return true
-}
-
 func (m *aclManager) createIPSet(name string) error {
 	opts := ipset.CreateOptions{
 		Replace: true,

client/firewall/iptables/ipset_support_linux.go

@@ -0,0 +1,75 @@
+package iptables
+
+import (
+	"sync"
+
+	log "github.com/sirupsen/logrus"
+)
+
+// ipsetSupport tracks whether ipset-backed firewall rules can be installed.
+//
+// It starts optimistic and latches to unsupported the first time the kernel
+// proves otherwise: either the hash:net set type is missing (ip_set_hash_net) or
+// iptables cannot match against a set (xt_set). Callers then emit per-IP and
+// per-prefix rules instead. Without the fallback, a rule referencing an unusable
+// set is never installed and the catch-all DROP silently blocks traffic the
+// policy permits.
+//
+// One instance is shared by the ACL managers and routers of both address
+// families, because ipset availability is a property of the kernel rather than
+// of any single table.
+type ipsetSupport struct {
+	mu          sync.RWMutex
+	unsupported bool
+}
+
+func newIPSetSupport() *ipsetSupport {
+	return &ipsetSupport{}
+}
+
+func (s *ipsetSupport) supported() bool {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+
+	return !s.unsupported
+}
+
+// markUnsupported records that ipset cannot be used, logging the reason once.
+func (s *ipsetSupport) markUnsupported(cause error) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	if s.unsupported {
+		return
+	}
+	s.unsupported = true
+
+	log.Warnf("ipset is unavailable (%v); falling back to per-IP firewall rules. "+
+		"Ensure the kernel provides ip_set_hash_net and xt_set; without them rule "+
+		"sets are larger and slower to converge on networks with many peers", cause)
+}
+
+// ipsetUnusableError marks a failure attributable to ipset, so the caller can
+// retry the same rule in its per-IP form before latching the capability off.
+type ipsetUnusableError struct {
+	cause error
+}
+
+func (e *ipsetUnusableError) Error() string { return e.cause.Error() }
+
+func (e *ipsetUnusableError) Unwrap() error { return e.cause }
+
+func ipsetUnusable(cause error) error {
+	return &ipsetUnusableError{cause: cause}
+}
+
+// maybeIPSetUnusable marks an iptables failure as ipset-attributable only when the
+// rule actually carried a set match, since the same call can fail for unrelated
+// reasons on a rule that matches addresses directly.
+func maybeIPSetUnusable(ipsetName string, err error) error {
+	if ipsetName == "" {
+		return err
+	}
+
+	return ipsetUnusable(err)
+}

client/firewall/iptables/manager_linux.go

@@ -33,6 +33,10 @@ type Manager struct {
 	router       *router
 	rawSupported bool
 
+	// ipsetSupport is shared by the ACL managers and routers of both families,
+	// so a kernel without usable ipset support degrades them together.
+	ipsetSupport *ipsetSupport
+
 	// IPv6 counterparts, nil when no v6 overlay
 	ipv6Client *iptables.IPTables
 	aclMgr6    *aclManager
@@ -53,16 +57,17 @@ func Create(wgIface iFaceMapper, mtu uint16) (*Manager, error) {
 	}
 
 	m := &Manager{
-		wgIface:    wgIface,
-		ipv4Client: iptablesClient,
+		wgIface:      wgIface,
+		ipv4Client:   iptablesClient,
+		ipsetSupport: newIPSetSupport(),
 	}
 
-	m.router, err = newRouter(iptablesClient, wgIface, mtu)
+	m.router, err = newRouter(iptablesClient, wgIface, mtu, m.ipsetSupport)
 	if err != nil {
 		return nil, fmt.Errorf("create router: %w", err)
 	}
 
-	m.aclMgr, err = newAclManager(iptablesClient, wgIface)
+	m.aclMgr, err = newAclManager(iptablesClient, wgIface, m.ipsetSupport)
 	if err != nil {
 		return nil, fmt.Errorf("create acl manager: %w", err)
 	}
@@ -83,7 +88,7 @@ func (m *Manager) createIPv6Components(wgIface iFaceMapper, mtu uint16) error {
 	}
 	m.ipv6Client = ip6Client
 
-	m.router6, err = newRouter(ip6Client, wgIface, mtu)
+	m.router6, err = newRouter(ip6Client, wgIface, mtu, m.ipsetSupport)
 	if err != nil {
 		return fmt.Errorf("create v6 router: %w", err)
 	}
@@ -92,7 +97,7 @@ func (m *Manager) createIPv6Components(wgIface iFaceMapper, mtu uint16) error {
 	// Forwarding refcounter is per-family but shared between v4 and v6 routers.
 	m.router6.ipFwdState = m.router.ipFwdState
 
-	m.aclMgr6, err = newAclManager(ip6Client, wgIface)
+	m.aclMgr6, err = newAclManager(ip6Client, wgIface, m.ipsetSupport)
 	if err != nil {
 		return fmt.Errorf("create v6 acl manager: %w", err)
 	}

client/firewall/iptables/manager_linux_test.go

@@ -292,39 +292,97 @@ func TestIptablesCreatePerformance(t *testing.T) {
 	}
 }
 
-// TestIptablesACLIPSetFallback verifies that when the kernel lacks ipset support,
-// the ACL manager falls back to per-IP iptables rules (-s <ip>) instead of
-// silently leaving the chain empty. See discussion #6125.
-func TestIptablesACLIPSetFallback(t *testing.T) {
-	ipv4Client, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
-	require.NoError(t, err)
+// newACLTestManager returns a started manager. Create()/Init() is used so the
+// router-owned chains (chainRTFWDIN/OUT) exist before the ACL manager's
+// createDefaultChains() references them.
+func newACLTestManager(t *testing.T) *Manager {
+	t.Helper()
 
-	// Use Create()/Init() so the router-owned chains (chainRTFWDIN/OUT) are
-	// created before the ACL manager's createDefaultChains() references them.
 	manager, err := Create(ifaceMock, iface.DefaultMTU)
 	require.NoError(t, err)
 	require.NoError(t, manager.Init(nil))
-
-	aclMgr := manager.aclMgr
-	// Simulate a kernel without the ipset hash module.
-	aclMgr.ipsetSupported = false
-
-	defer func() {
+	t.Cleanup(func() {
 		require.NoError(t, manager.Close(nil))
-	}()
+	})
+
+	return manager
+}
+
+// TestIptablesACLUsesIPSetOnHealthyKernel guards the default: on a kernel that
+// does have ipset, rules must keep matching a set. A regression that reported
+// ipset as unusable would silently move every Linux client to per-IP rules.
+func TestIptablesACLUsesIPSetOnHealthyKernel(t *testing.T) {
+	ipv4Client, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
+	require.NoError(t, err)
+
+	manager := newACLTestManager(t)
 
 	ip := netip.MustParseAddr("10.20.0.42")
 	port := &fw.Port{Values: []uint16{22}}
 
-	rules, err := aclMgr.AddPeerFiltering(nil, ip.AsSlice(), "tcp", nil, port, fw.ActionAccept, "nb0000001")
-	require.NoError(t, err, "AddPeerFiltering should succeed via fallback")
+	rules, err := manager.aclMgr.AddPeerFiltering(nil, ip.AsSlice(), "tcp", nil, port, fw.ActionAccept, "nb0000001")
+	require.NoError(t, err)
+	require.NotEmpty(t, rules)
+
+	rule := rules[0].(*Rule)
+	require.Equal(t, "nb0000001-dport", rule.ipsetName, "healthy kernel must use an ipset")
+	require.Contains(t, rule.specs, "--match-set")
+	require.True(t, manager.ipsetSupport.supported(), "ipset must not be latched off on a healthy kernel")
+
+	checkRuleSpecs(t, ipv4Client, rule.chain, true, rule.specs...)
+}
+
+// TestIptablesACLFallsBackWhenIPSetUnusable drives the real failure path: an
+// oversized set name is rejected by the kernel, which stands in for a kernel
+// without ip_set_hash_net or xt_set. The rule must still land in the chain,
+// matching the IP directly, and the capability must latch off so later rules skip
+// ipset. Before the fallback existed, the rule was dropped and the catch-all DROP
+// silently blocked traffic the policy permits.
+func TestIptablesACLFallsBackWhenIPSetUnusable(t *testing.T) {
+	ipv4Client, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
+	require.NoError(t, err)
+
+	manager := newACLTestManager(t)
+
+	// ipset names are limited to 31 characters, so creating this set fails.
+	unusableName := strings.Repeat("a", 40)
+
+	ip := netip.MustParseAddr("10.20.0.42")
+	port := &fw.Port{Values: []uint16{22}}
+
+	rules, err := manager.aclMgr.AddPeerFiltering(nil, ip.AsSlice(), "tcp", nil, port, fw.ActionAccept, unusableName)
+	require.NoError(t, err, "AddPeerFiltering must succeed by falling back")
 	require.NotEmpty(t, rules)
 
 	rule := rules[0].(*Rule)
 	require.Empty(t, rule.ipsetName, "fallback rule must not reference an ipset")
-	require.Contains(t, strings.Join(rule.specs, " "), "-s 10.20.0.42", "fallback rule must match by source IP")
-	require.NotContains(t, strings.Join(rule.specs, " "), "--match-set", "fallback rule must not use ipset matching")
+	require.Contains(t, strings.Join(rule.specs, " "), "-s 10.20.0.42", "fallback rule must match the source IP")
+	require.NotContains(t, strings.Join(rule.specs, " "), "--match-set")
 
-	// The rule must actually be present in the ACL chain (not silently dropped).
+	// The rule must actually be present, not silently missing.
 	checkRuleSpecs(t, ipv4Client, rule.chain, true, rule.specs...)
+
+	require.False(t, manager.ipsetSupport.supported(), "failure must latch ipset off")
+
+	// A subsequent rule with a perfectly valid set name now skips ipset too.
+	next, err := manager.aclMgr.AddPeerFiltering(nil, netip.MustParseAddr("10.20.0.43").AsSlice(), "tcp", nil, port, fw.ActionAccept, "nb0000001")
+	require.NoError(t, err)
+	require.NotEmpty(t, next)
+	require.Empty(t, next[0].(*Rule).ipsetName, "later rules must skip ipset once latched")
+}
+
+// TestIptablesACLLeavesNoIPSetAfterFallback verifies the set created before the
+// failure is destroyed, so a later rule does not find a half-built set and assume
+// ipset works.
+func TestIptablesACLLeavesNoIPSetAfterFallback(t *testing.T) {
+	manager := newACLTestManager(t)
+
+	port := &fw.Port{Values: []uint16{22}}
+	ip := netip.MustParseAddr("10.20.0.42")
+
+	_, err := manager.aclMgr.AddPeerFiltering(nil, ip.AsSlice(), "tcp", nil, port, fw.ActionAccept, strings.Repeat("a", 40))
+	require.NoError(t, err)
+
+	_, exists := manager.aclMgr.ipsetStore.ipset(strings.Repeat("a", 40) + "-dport")
+	require.False(t, exists, "unusable set must not stay in the store")
 }

client/firewall/iptables/router_linux.go

@@ -3,6 +3,7 @@
 package iptables
 
 import (
+	"errors"
 	"fmt"
 	"maps"
 	"net/netip"
@@ -51,6 +52,10 @@ const (
 	markManglePost = "mark-mangle-post"
 	matchSet       = "--match-set"
 
+	// routeSourceSuffix names the extra rules a route ACL needs when ipset is
+	// unusable and each source prefix has to be matched by its own rule.
+	routeSourceSuffix = "_src"
+
 	dnatSuffix = "_dnat"
 	snatSuffix = "_snat"
 	fwdSuffix  = "_fwd"
@@ -68,7 +73,6 @@ type ruleInfo struct {
 }
 
 type routeFilteringRuleParams struct {
-	Source      firewall.Network
 	Destination firewall.Network
 	Proto       firewall.Protocol
 	SPort       *firewall.Port
@@ -90,12 +94,13 @@ type router struct {
 	legacyManagement bool
 	mtu              uint16
 	v6               bool
+	ipsetSupport     *ipsetSupport
 
 	stateManager *statemanager.Manager
 	ipFwdState   *ipfwdstate.IPForwardingState
 }
 
-func newRouter(iptablesClient *iptables.IPTables, wgIface iFaceMapper, mtu uint16) (*router, error) {
+func newRouter(iptablesClient *iptables.IPTables, wgIface iFaceMapper, mtu uint16, ipsetSupport *ipsetSupport) (*router, error) {
 	r := &router{
 		iptablesClient: iptablesClient,
 		rules:          make(map[string][]string),
@@ -103,6 +108,7 @@ func newRouter(iptablesClient *iptables.IPTables, wgIface iFaceMapper, mtu uint1
 		mtu:            mtu,
 		v6:             iptablesClient.Proto() == iptables.ProtocolIPv6,
 		ipFwdState:     ipfwdstate.NewIPForwardingState(wgIface.Name()),
+		ipsetSupport:   ipsetSupport,
 	}
 
 	r.ipsetCounter = refcounter.New(
@@ -151,15 +157,7 @@ func (r *router) AddRouteFiltering(
 		return ruleKey, nil
 	}
 
-	var source firewall.Network
-	if len(sources) > 1 {
-		source.Set = firewall.NewPrefixSet(sources)
-	} else if len(sources) > 0 {
-		source.Prefix = sources[0]
-	}
-
 	params := routeFilteringRuleParams{
-		Source:      source,
 		Destination: destination,
 		Proto:       proto,
 		SPort:       sPort,
@@ -167,30 +165,156 @@ func (r *router) AddRouteFiltering(
 		Action:      action,
 	}
 
-	rule, err := r.genRouteRuleSpec(params, sources)
-	if err != nil {
-		return nil, fmt.Errorf("generate route rule spec: %w", err)
-	}
+	err := r.installRouteRules(string(ruleKey), params, sources, r.ipsetSupport.supported())
 
-	// Insert DROP rules at the beginning, append ACCEPT rules at the end
-	if action == firewall.ActionDrop {
-		// after the established rule
-		err = r.iptablesClient.Insert(tableFilter, chainRTFWDIN, 2, rule...)
-	} else {
-		err = r.iptablesClient.Append(tableFilter, chainRTFWDIN, rule...)
+	var unusable *ipsetUnusableError
+	if errors.As(err, &unusable) {
+		// The set could not be created or matched. Retry matching each source
+		// prefix on its own; only if that works do we know ipset was to blame.
+		r.removeRouteRules(string(ruleKey))
+
+		if retryErr := r.installRouteRules(string(ruleKey), params, sources, false); retryErr != nil {
+			r.removeRouteRules(string(ruleKey))
+			return nil, fmt.Errorf("add route rule (ipset: %w): %w", unusable.cause, retryErr)
+		}
+
+		r.ipsetSupport.markUnsupported(unusable.cause)
+		err = nil
 	}
 
 	if err != nil {
-		return nil, fmt.Errorf("add route rule: %v", err)
-	}
+		// Leave nothing half-installed: a later call finding the rule key would
+		// report success while some sources were never installed, which for a
+		// drop rule would leave them unblocked.
+		r.removeRouteRules(string(ruleKey))
 
-	r.rules[string(ruleKey)] = rule
+		return nil, fmt.Errorf("add route rule: %w", err)
+	}
 
 	r.updateState()
 
 	return ruleKey, nil
 }
 
+// installRouteRules installs every rule needed for one route ACL and records them
+// under ruleKey. It is more than one rule only when useIPSet is false and the
+// sources have to be matched one prefix at a time.
+func (r *router) installRouteRules(ruleKey string, params routeFilteringRuleParams, sources []netip.Prefix, useIPSet bool) error {
+	specs, err := r.genRouteRuleSpecs(params, sources, useIPSet)
+	if err != nil {
+		return fmt.Errorf("generate route rule spec: %w", err)
+	}
+
+	for i, spec := range specs {
+		if err := r.insertRouteRule(params.Action, spec); err != nil {
+			if len(r.findSets(spec)) > 0 {
+				return ipsetUnusable(err)
+			}
+			return err
+		}
+		r.rules[routeRuleKey(ruleKey, i)] = spec
+	}
+
+	return nil
+}
+
+// genRouteRuleSpecs builds the rules for one route ACL. With ipset available that
+// is a single rule matching a set of sources; without it, one rule per source
+// prefix, which is the only form a stripped kernel can express.
+func (r *router) genRouteRuleSpecs(params routeFilteringRuleParams, sources []netip.Prefix, useIPSet bool) ([][]string, error) {
+	destExp, err := r.applyNetwork("-d", params.Destination, nil)
+	if err != nil {
+		return nil, fmt.Errorf("apply network -d: %w", err)
+	}
+
+	specs, err := r.genSourceRules(params, sources, useIPSet, destExp)
+	if err != nil {
+		// The destination match may have taken a set reference already.
+		if decErr := r.decrementSetCounter(destExp); decErr != nil {
+			log.Debugf("release destination set after failed rule generation: %v", decErr)
+		}
+
+		return nil, err
+	}
+
+	return specs, nil
+}
+
+func (r *router) genSourceRules(params routeFilteringRuleParams, sources []netip.Prefix, useIPSet bool, destExp []string) ([][]string, error) {
+	if useIPSet || len(sources) <= 1 {
+		sourceExp, err := r.applyNetwork("-s", sourceNetwork(sources), sources)
+		if err != nil {
+			return nil, fmt.Errorf("apply network -s: %w", err)
+		}
+
+		return [][]string{assembleRouteRule(sourceExp, destExp, params, r.v6)}, nil
+	}
+
+	specs := make([][]string, 0, len(sources))
+	for _, source := range sources {
+		sourceExp, err := r.applyNetwork("-s", firewall.Network{Prefix: source}, nil)
+		if err != nil {
+			return nil, fmt.Errorf("apply network -s: %w", err)
+		}
+
+		specs = append(specs, assembleRouteRule(sourceExp, destExp, params, r.v6))
+	}
+
+	return specs, nil
+}
+
+func sourceNetwork(sources []netip.Prefix) firewall.Network {
+	var source firewall.Network
+	if len(sources) > 1 {
+		source.Set = firewall.NewPrefixSet(sources)
+	} else if len(sources) > 0 {
+		source.Prefix = sources[0]
+	}
+
+	return source
+}
+
+func (r *router) insertRouteRule(action firewall.Action, spec []string) error {
+	// Insert DROP rules at the beginning, append ACCEPT rules at the end
+	if action == firewall.ActionDrop {
+		// after the established rule
+		return r.iptablesClient.Insert(tableFilter, chainRTFWDIN, 2, spec...)
+	}
+
+	return r.iptablesClient.Append(tableFilter, chainRTFWDIN, spec...)
+}
+
+// removeRouteRules deletes the rules recorded for ruleKey, used to undo a partial
+// install before retrying without ipset.
+func (r *router) removeRouteRules(ruleKey string) {
+	for i := 0; ; i++ {
+		key := routeRuleKey(ruleKey, i)
+		spec, exists := r.rules[key]
+		if !exists {
+			return
+		}
+
+		if err := r.iptablesClient.DeleteIfExists(tableFilter, chainRTFWDIN, spec...); err != nil {
+			log.Debugf("delete partial route rule %s: %v", key, err)
+		}
+		delete(r.rules, key)
+
+		if err := r.decrementSetCounter(spec); err != nil {
+			log.Debugf("decrement ipset counter for %s: %v", key, err)
+		}
+	}
+}
+
+// routeRuleKey names the i-th rule of a route ACL. The first keeps the plain rule
+// key so single-rule ACLs, which is every ACL when ipset works, are unaffected.
+func routeRuleKey(ruleKey string, i int) string {
+	if i == 0 {
+		return ruleKey
+	}
+
+	return fmt.Sprintf("%s%s%d", ruleKey, routeSourceSuffix, i)
+}
+
 func (r *router) hasRule(id string) bool {
 	_, ok := r.rules[id]
 	return ok
@@ -199,17 +323,29 @@ func (r *router) hasRule(id string) bool {
 func (r *router) DeleteRouteRule(rule firewall.Rule) error {
 	ruleKey := rule.ID()
 
-	if rule, exists := r.rules[ruleKey]; exists {
+	if _, exists := r.rules[ruleKey]; !exists {
+		log.Debugf("route rule %s not found", ruleKey)
+		r.updateState()
+
+		return nil
+	}
+
+	// In the ipset fallback one ACL is several rules, one per source prefix.
+	for i := 0; ; i++ {
+		key := routeRuleKey(ruleKey, i)
+		rule, exists := r.rules[key]
+		if !exists {
+			break
+		}
+
 		if err := r.iptablesClient.Delete(tableFilter, chainRTFWDIN, rule...); err != nil {
 			return fmt.Errorf("delete route rule: %v", err)
 		}
-		delete(r.rules, ruleKey)
+		delete(r.rules, key)
 
 		if err := r.decrementSetCounter(rule); err != nil {
 			return fmt.Errorf("decrement ipset counter: %w", err)
 		}
-	} else {
-		log.Debugf("route rule %s not found", ruleKey)
 	}
 
 	r.updateState()
@@ -927,31 +1063,23 @@ func (r *router) DeleteDNATRule(rule firewall.Rule) error {
 	return nberrors.FormatErrorOrNil(merr)
 }
 
-func (r *router) genRouteRuleSpec(params routeFilteringRuleParams, sources []netip.Prefix) ([]string, error) {
+// assembleRouteRule joins the pre-built source and destination matches with the
+// protocol, ports and target of a route ACL.
+func assembleRouteRule(sourceExp, destExp []string, params routeFilteringRuleParams, v6 bool) []string {
 	var rule []string
 
-	sourceExp, err := r.applyNetwork("-s", params.Source, sources)
-	if err != nil {
-		return nil, fmt.Errorf("apply network -s: %w", err)
-
-	}
-	destExp, err := r.applyNetwork("-d", params.Destination, nil)
-	if err != nil {
-		return nil, fmt.Errorf("apply network -d: %w", err)
-	}
-
 	rule = append(rule, sourceExp...)
 	rule = append(rule, destExp...)
 
 	if params.Proto != firewall.ProtocolALL {
-		rule = append(rule, "-p", strings.ToLower(protoForFamily(params.Proto, r.v6)))
+		rule = append(rule, "-p", strings.ToLower(protoForFamily(params.Proto, v6)))
 		rule = append(rule, applyPort("--sport", params.SPort)...)
 		rule = append(rule, applyPort("--dport", params.DPort)...)
 	}
 
 	rule = append(rule, "-j", actionToStr(params.Action))
 
-	return rule, nil
+	return rule
 }
 
 func (r *router) applyNetwork(flag string, network firewall.Network, prefixes []netip.Prefix) ([]string, error) {
@@ -961,9 +1089,17 @@ func (r *router) applyNetwork(flag string, network firewall.Network, prefixes []
 	}
 
 	if network.IsSet() {
+		// A destination set is populated later from DNS results, so unlike a
+		// source set it cannot be expanded into per-prefix rules here. Without
+		// ipset such a rule is not expressible; report it instead of installing
+		// something broader than the policy allows.
+		if flag == "-d" && !r.ipsetSupport.supported() {
+			return nil, fmt.Errorf("destination set %s requires ipset (ip_set_hash_net and xt_set)", network.Set.HashedName())
+		}
+
 		name := r.ipsetName(network.Set.HashedName())
 		if _, err := r.ipsetCounter.Increment(name, prefixes); err != nil {
-			return nil, fmt.Errorf("create or get ipset: %w", err)
+			return nil, ipsetUnusable(fmt.Errorf("create or get ipset: %w", err))
 		}
 
 		return []string{"-m", "set", matchSet, name, direction}, nil

client/firewall/iptables/router_linux_test.go

@@ -3,9 +3,11 @@
 package iptables
 
 import (
+	"errors"
 	"fmt"
 	"net/netip"
 	"os/exec"
+	"strings"
 	"testing"
 
 	"github.com/coreos/go-iptables/iptables"
@@ -15,7 +17,9 @@ import (
 	firewall "github.com/netbirdio/netbird/client/firewall/manager"
 	"github.com/netbirdio/netbird/client/firewall/test"
 	"github.com/netbirdio/netbird/client/iface"
+	nbid "github.com/netbirdio/netbird/client/internal/acl/id"
 	nbnet "github.com/netbirdio/netbird/client/net"
+	"github.com/netbirdio/netbird/shared/management/domain"
 )
 
 func isIptablesSupported() bool {
@@ -31,7 +35,7 @@ func TestIptablesManager_RestoreOrCreateContainers(t *testing.T) {
 	iptablesClient, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
 	require.NoError(t, err, "failed to init iptables client")
 
-	manager, err := newRouter(iptablesClient, ifaceMock, iface.DefaultMTU)
+	manager, err := newRouter(iptablesClient, ifaceMock, iface.DefaultMTU, newIPSetSupport())
 	require.NoError(t, err, "should return a valid iptables manager")
 	require.NoError(t, manager.init(nil))
 
@@ -84,7 +88,7 @@ func TestIptablesManager_AddNatRule(t *testing.T) {
 			iptablesClient, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
 			require.NoError(t, err, "failed to init iptables client")
 
-			manager, err := newRouter(iptablesClient, ifaceMock, iface.DefaultMTU)
+			manager, err := newRouter(iptablesClient, ifaceMock, iface.DefaultMTU, newIPSetSupport())
 			require.NoError(t, err, "shouldn't return error")
 			require.NoError(t, manager.init(nil))
 
@@ -157,7 +161,7 @@ func TestIptablesManager_RemoveNatRule(t *testing.T) {
 		t.Run(testCase.Name, func(t *testing.T) {
 			iptablesClient, _ := iptables.NewWithProtocol(iptables.ProtocolIPv4)
 
-			manager, err := newRouter(iptablesClient, ifaceMock, iface.DefaultMTU)
+			manager, err := newRouter(iptablesClient, ifaceMock, iface.DefaultMTU, newIPSetSupport())
 			require.NoError(t, err, "shouldn't return error")
 			require.NoError(t, manager.init(nil))
 			defer func() {
@@ -219,7 +223,7 @@ func TestRouter_AddRouteFiltering(t *testing.T) {
 	iptablesClient, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
 	require.NoError(t, err, "Failed to create iptables client")
 
-	r, err := newRouter(iptablesClient, ifaceMock, iface.DefaultMTU)
+	r, err := newRouter(iptablesClient, ifaceMock, iface.DefaultMTU, newIPSetSupport())
 	require.NoError(t, err, "Failed to create router manager")
 	require.NoError(t, r.init(nil))
 
@@ -337,27 +341,26 @@ func TestRouter_AddRouteFiltering(t *testing.T) {
 			ruleKey, err := r.AddRouteFiltering(nil, tt.sources, firewall.Network{Prefix: tt.destination}, tt.proto, tt.sPort, tt.dPort, tt.action)
 			require.NoError(t, err, "AddRouteFiltering failed")
 
-			// Check if the rule is in the internal map
-			rule, ok := r.rules[ruleKey.ID()]
-			assert.True(t, ok, "Rule not found in internal map")
+			// A kernel without usable ipset splits a multi-source ACL into one
+			// rule per source, so compare against whichever form is in effect.
+			useIPSet := r.ipsetSupport.supported()
 
-			// Log the internal rule
-			t.Logf("Internal rule: %v", rule)
+			// Check if the rules are in the internal map
+			rules := routeRuleSpecs(t, r, ruleKey.ID())
+			require.NotEmpty(t, rules, "Rule not found in internal map")
 
-			// Check if the rule exists in iptables
-			exists, err := iptablesClient.Exists(tableFilter, chainRTFWDIN, rule...)
-			assert.NoError(t, err, "Failed to check rule existence")
-			assert.True(t, exists, "Rule not found in iptables")
+			// Log the internal rules
+			t.Logf("Internal rules: %v", rules)
 
-			var source firewall.Network
-			if len(tt.sources) > 1 {
-				source.Set = firewall.NewPrefixSet(tt.sources)
-			} else if len(tt.sources) > 0 {
-				source.Prefix = tt.sources[0]
+			// Check if the rules exist in iptables
+			for _, rule := range rules {
+				exists, err := iptablesClient.Exists(tableFilter, chainRTFWDIN, rule...)
+				assert.NoError(t, err, "Failed to check rule existence")
+				assert.True(t, exists, "Rule not found in iptables")
 			}
+
 			// Verify rule content
 			params := routeFilteringRuleParams{
-				Source:      source,
 				Destination: firewall.Network{Prefix: tt.destination},
 				Proto:       tt.proto,
 				SPort:       tt.sPort,
@@ -365,20 +368,18 @@ func TestRouter_AddRouteFiltering(t *testing.T) {
 				Action:      tt.action,
 			}
 
-			expectedRule, err := r.genRouteRuleSpec(params, nil)
+			expectedRules, err := r.genRouteRuleSpecs(params, tt.sources, useIPSet)
 			require.NoError(t, err, "Failed to generate expected rule spec")
 
-			if tt.expectSet {
+			if tt.expectSet && useIPSet {
 				setName := firewall.NewPrefixSet(tt.sources).HashedName()
-				expectedRule, err = r.genRouteRuleSpec(params, nil)
-				require.NoError(t, err, "Failed to generate expected rule spec with set")
 
 				// Check if the set was created
 				_, exists := r.ipsetCounter.Get(setName)
 				assert.True(t, exists, "IPSet not created")
 			}
 
-			assert.Equal(t, expectedRule, rule, "Rule content mismatch")
+			assert.Equal(t, expectedRules, rules, "Rule content mismatch")
 
 			// Clean up
 			err = r.DeleteRouteRule(ruleKey)
@@ -445,3 +446,145 @@ func TestFindSetNameInRule(t *testing.T) {
 		})
 	}
 }
+
+// TestRouter_AddRouteFilteringIPSetFallback covers a kernel that cannot use ipset:
+// a multi-source route ACL must become one rule per source prefix, all present in
+// the chain, and deleting the ACL must remove every one of them. Without the
+// fallback the rule was never installed and the interface-wide DROP in FORWARD
+// silently dropped routed traffic.
+func TestRouter_AddRouteFilteringIPSetFallback(t *testing.T) {
+	if !isIptablesSupported() {
+		t.Skip("iptables not supported on this system")
+	}
+
+	iptablesClient, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
+	require.NoError(t, err)
+
+	support := newIPSetSupport()
+	support.markUnsupported(errors.New("test: pretend the kernel has no ipset"))
+
+	r, err := newRouter(iptablesClient, ifaceMock, iface.DefaultMTU, support)
+	require.NoError(t, err)
+	require.NoError(t, r.init(nil))
+	t.Cleanup(func() {
+		require.NoError(t, r.Reset())
+	})
+
+	sources := []netip.Prefix{
+		netip.MustParsePrefix("172.16.0.0/16"),
+		netip.MustParsePrefix("192.168.0.0/16"),
+	}
+	destination := firewall.Network{Prefix: netip.MustParsePrefix("10.0.0.0/8")}
+
+	rule, err := r.AddRouteFiltering(nil, sources, destination, firewall.ProtocolTCP, nil,
+		&firewall.Port{Values: []uint16{443}}, firewall.ActionAccept)
+	require.NoError(t, err, "route ACL must install without ipset")
+
+	specs := routeRuleSpecs(t, r, rule.ID())
+	require.Len(t, specs, len(sources), "each source prefix needs its own rule")
+
+	for i, spec := range specs {
+		joined := strings.Join(spec, " ")
+		require.Contains(t, joined, "-s "+sources[i].String(), "rule must match the source prefix directly")
+		require.NotContains(t, joined, matchSet, "fallback rule must not reference a set")
+
+		exists, err := iptablesClient.Exists(tableFilter, chainRTFWDIN, spec...)
+		require.NoError(t, err)
+		require.True(t, exists, "rule %d must be present in %s", i, chainRTFWDIN)
+	}
+
+	require.NoError(t, r.DeleteRouteRule(rule))
+
+	for i, spec := range specs {
+		exists, err := iptablesClient.Exists(tableFilter, chainRTFWDIN, spec...)
+		require.NoError(t, err)
+		require.False(t, exists, "rule %d must be removed", i)
+	}
+	require.Empty(t, routeRuleSpecs(t, r, rule.ID()), "no rule may be left recorded")
+}
+
+// TestRouter_DestinationSetRequiresIPSet documents that a dynamic (domain)
+// destination cannot be expressed without ipset: its prefixes are only known
+// after DNS resolution, so there is nothing to expand into per-prefix rules. The
+// call must report that rather than install a broader rule than the policy allows.
+func TestRouter_DestinationSetRequiresIPSet(t *testing.T) {
+	if !isIptablesSupported() {
+		t.Skip("iptables not supported on this system")
+	}
+
+	iptablesClient, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
+	require.NoError(t, err)
+
+	support := newIPSetSupport()
+	support.markUnsupported(errors.New("test: pretend the kernel has no ipset"))
+
+	r, err := newRouter(iptablesClient, ifaceMock, iface.DefaultMTU, support)
+	require.NoError(t, err)
+	require.NoError(t, r.init(nil))
+	t.Cleanup(func() {
+		require.NoError(t, r.Reset())
+	})
+
+	destination := firewall.Network{Set: firewall.NewDomainSet(domain.List{"example.com"})}
+
+	_, err = r.AddRouteFiltering(nil, []netip.Prefix{netip.MustParsePrefix("172.16.0.0/16")},
+		destination, firewall.ProtocolALL, nil, nil, firewall.ActionAccept)
+	require.Error(t, err, "a domain destination is not expressible without ipset")
+	require.ErrorContains(t, err, "requires ipset")
+}
+
+// TestRouter_RouteFilteringRollsBackPartialInstall covers a fallback ACL whose
+// second rule cannot be installed. Nothing may be left behind: if the rule key
+// survived, a later call would short-circuit on it and report success while some
+// sources were never installed, leaving them unblocked for a drop rule.
+func TestRouter_RouteFilteringRollsBackPartialInstall(t *testing.T) {
+	if !isIptablesSupported() {
+		t.Skip("iptables not supported on this system")
+	}
+
+	iptablesClient, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
+	require.NoError(t, err)
+
+	support := newIPSetSupport()
+	support.markUnsupported(errors.New("test: pretend the kernel has no ipset"))
+
+	r, err := newRouter(iptablesClient, ifaceMock, iface.DefaultMTU, support)
+	require.NoError(t, err)
+	require.NoError(t, r.init(nil))
+	t.Cleanup(func() {
+		require.NoError(t, r.Reset())
+	})
+
+	// The v6 prefix is rejected by the v4 iptables binary, so the second rule of
+	// the expansion fails after the first has been installed.
+	good := netip.MustParsePrefix("172.16.0.0/16")
+	sources := []netip.Prefix{good, netip.MustParsePrefix("2001:db8::/32")}
+	destination := firewall.Network{Prefix: netip.MustParsePrefix("10.0.0.0/8")}
+
+	_, err = r.AddRouteFiltering(nil, sources, destination, firewall.ProtocolALL, nil, nil, firewall.ActionDrop)
+	require.Error(t, err, "a source that iptables rejects must fail the whole ACL")
+
+	ruleKey := nbid.GenerateRouteRuleKey(sources, destination, firewall.ProtocolALL, nil, nil, firewall.ActionDrop)
+	require.Empty(t, routeRuleSpecs(t, r, string(ruleKey)), "no rule may stay recorded")
+
+	// The rule that did get installed must be gone from the chain.
+	installed := []string{"-s", good.String(), "-d", "10.0.0.0/8", "-j", "DROP"}
+	exists, err := iptablesClient.Exists(tableFilter, chainRTFWDIN, installed...)
+	require.NoError(t, err)
+	require.False(t, exists, "the already-installed rule must be rolled back")
+}
+
+// routeRuleSpecs collects the rules recorded for one route ACL, which is more than
+// one when the ipset fallback splits it per source prefix.
+func routeRuleSpecs(t *testing.T, r *router, ruleKey string) [][]string {
+	t.Helper()
+
+	var specs [][]string
+	for i := 0; ; i++ {
+		spec, exists := r.rules[routeRuleKey(ruleKey, i)]
+		if !exists {
+			return specs
+		}
+		specs = append(specs, spec)
+	}
+}