Remove partial userspace firewall mode and open foreign chains via a table-less allower

client/firewall/nftables/chains_linux.go

@@ -83,8 +83,9 @@ func (r *family) createContainers() error {
 		log.Errorf("failed to add MSS clamping rules: %s", err)
 	}
 
-	if err := r.acceptForwardRules(); err != nil {
-		log.Errorf("failed to add accept rules for the forward chain: %s", err)
+	// Kernel routing opens both INPUT and FORWARD.
+	if err := r.openInterface(true); err != nil {
+		log.Errorf("failed to open interface in foreign chains: %s", err)
 	}
 
 	if err := firewalld.TrustInterface(r.wgIface.Name()); err != nil {
@@ -174,21 +175,26 @@ func (r *family) setupDataPlaneMark() error {
 	return nil
 }
 
-func (r *family) acceptForwardRules() error {
+// openInterface adds passthrough accept rules for the NetBird interface to the
+// kernel's filter table and external chains so they don't drop our traffic.
+// includeForward also opens the FORWARD chains (kernel routing); when false only
+// INPUT is opened, which is all the userspace router needs since it never
+// forwards in the kernel.
+func (r *family) openInterface(includeForward bool) error {
 	var merr *multierror.Error
 
-	if err := r.acceptFilterTableRules(); err != nil {
+	if err := r.acceptFilterTableRules(includeForward); err != nil {
 		merr = multierror.Append(merr, err)
 	}
 
-	if err := r.acceptExternalChainsRules(); err != nil {
+	if err := r.acceptExternalChainsRules(includeForward); err != nil {
 		merr = multierror.Append(merr, fmt.Errorf("add accept rules to external chains: %w", err))
 	}
 
 	return nberrors.FormatErrorOrNil(merr)
 }
 
-func (r *family) acceptFilterTableRules() error {
+func (r *family) acceptFilterTableRules(includeForward bool) error {
 	if r.filterTable == nil {
 		return nil
 	}
@@ -196,7 +202,7 @@ func (r *family) acceptFilterTableRules() error {
 	fw := "iptables"
 
 	defer func() {
-		log.Debugf("Used %s to add accept forward and input rules", fw)
+		log.Debugf("Used %s to add accept input/forward rules", fw)
 	}()
 
 	// Try iptables first and fallback to nftables if iptables is not available.
@@ -206,25 +212,27 @@ func (r *family) acceptFilterTableRules() error {
 		log.Warnf("Will use nftables to manipulate the filter table because iptables is not available: %v", err)
 
 		fw = "nftables"
-		return r.acceptFilterRulesNftables(r.filterTable)
+		return r.acceptFilterRulesNftables(r.filterTable, includeForward)
 	}
 
-	if err := r.acceptFilterRulesIptables(ipt); err != nil {
+	if err := r.acceptFilterRulesIptables(ipt, includeForward); err != nil {
 		log.Warnf("iptables failed (table may be incompatible), falling back to nftables: %v", err)
 		fw = "nftables"
-		return r.acceptFilterRulesNftables(r.filterTable)
+		return r.acceptFilterRulesNftables(r.filterTable, includeForward)
 	}
 	return nil
 }
 
-func (r *family) acceptFilterRulesIptables(ipt *iptables.IPTables) error {
+func (r *family) acceptFilterRulesIptables(ipt *iptables.IPTables, includeForward bool) error {
 	var merr *multierror.Error
 
-	for _, rule := range r.getAcceptForwardRules() {
-		if err := ipt.Insert("filter", chainNameForward, 1, rule...); err != nil {
-			merr = multierror.Append(merr, fmt.Errorf("add iptables forward rule: %v", err))
-		} else {
-			log.Debugf("added iptables forward rule: %v", rule)
+	if includeForward {
+		for _, rule := range r.getAcceptForwardRules() {
+			if err := ipt.Insert("filter", chainNameForward, 1, rule...); err != nil {
+				merr = multierror.Append(merr, fmt.Errorf("add iptables forward rule: %v", err))
+			} else {
+				log.Debugf("added iptables forward rule: %v", rule)
+			}
 		}
 	}
 
@@ -252,17 +260,19 @@ func (r *family) getAcceptInputRule() []string {
 
 // acceptFilterRulesNftables adds accept rules to the ip filter table using nftables.
 // This is used when iptables is not available.
-func (r *family) acceptFilterRulesNftables(table *nftables.Table) error {
+func (r *family) acceptFilterRulesNftables(table *nftables.Table, includeForward bool) error {
 	intf := ifname(r.wgIface.Name())
 
-	forwardChain := &nftables.Chain{
-		Name:     chainNameForward,
-		Table:    table,
-		Type:     nftables.ChainTypeFilter,
-		Hooknum:  nftables.ChainHookForward,
-		Priority: nftables.ChainPriorityFilter,
+	if includeForward {
+		forwardChain := &nftables.Chain{
+			Name:     chainNameForward,
+			Table:    table,
+			Type:     nftables.ChainTypeFilter,
+			Hooknum:  nftables.ChainHookForward,
+			Priority: nftables.ChainPriorityFilter,
+		}
+		r.insertForwardAcceptRules(forwardChain, intf)
 	}
-	r.insertForwardAcceptRules(forwardChain, intf)
 
 	inputChain := &nftables.Chain{
 		Name:     chainNameInput,
@@ -278,7 +288,7 @@ func (r *family) acceptFilterRulesNftables(table *nftables.Table) error {
 
 // acceptExternalChainsRules adds accept rules to external chains (non-netbird, non-iptables tables).
 // It dynamically finds chains at call time to handle chains that may have been created after startup.
-func (r *family) acceptExternalChainsRules() error {
+func (r *family) acceptExternalChainsRules(includeForward bool) error {
 	chains := r.findExternalChains()
 	if len(chains) == 0 {
 		return nil
@@ -286,7 +296,7 @@ func (r *family) acceptExternalChainsRules() error {
 
 	intf := ifname(r.wgIface.Name())
 	for _, chain := range chains {
-		r.applyExternalChainAccept(chain, intf)
+		r.applyExternalChainAccept(chain, intf, includeForward)
 	}
 
 	if err := r.conn.Flush(); err != nil {
@@ -295,7 +305,7 @@ func (r *family) acceptExternalChainsRules() error {
 	return nil
 }
 
-func (r *family) applyExternalChainAccept(chain *nftables.Chain, intf []byte) {
+func (r *family) applyExternalChainAccept(chain *nftables.Chain, intf []byte, includeForward bool) {
 	if chain.Hooknum == nil {
 		log.Debugf("skipping external chain %s/%s: hooknum is nil", chain.Table.Name, chain.Name)
 		return
@@ -306,7 +316,9 @@ func (r *family) applyExternalChainAccept(chain *nftables.Chain, intf []byte) {
 
 	switch *chain.Hooknum {
 	case *nftables.ChainHookForward:
-		r.insertForwardAcceptRules(chain, intf)
+		if includeForward {
+			r.insertForwardAcceptRules(chain, intf)
+		}
 	case *nftables.ChainHookInput:
 		r.insertInputAcceptRule(chain, intf)
 	}
@@ -576,26 +588,6 @@ func (r *family) removeAcceptFilterRulesIptables(ipt *iptables.IPTables) error {
 	return nberrors.FormatErrorOrNil(merr)
 }
 
-func (r *family) createDefaultAllowRules() error {
-	expIn := []expr.Any{
-		&expr.Verdict{
-			Kind: expr.VerdictAccept,
-		},
-	}
-
-	_ = r.conn.InsertRule(&nftables.Rule{
-		Table:    r.workTable,
-		Chain:    r.chainInputRules,
-		Position: 0,
-		Exprs:    expIn,
-	})
-
-	if err := r.conn.Flush(); err != nil {
-		return fmt.Errorf(flushError, err)
-	}
-	return nil
-}
-
 // Flush rule/chain/set operations from the buffer
 //
 // Method also get all rules after flush and refreshes handle values in the rulesets

client/firewall/nftables/interface_allower_integration_linux_test.go

@@ -0,0 +1,90 @@
+//go:build integration && !android
+
+package nftables
+
+import (
+	"bytes"
+	"os"
+	"testing"
+
+	"github.com/google/nftables"
+	"github.com/stretchr/testify/require"
+
+	"github.com/netbirdio/netbird/client/iface"
+)
+
+// TestInterfaceAllowerInputOnly verifies the userspace-mode allower opens the
+// interface on the INPUT hook of foreign chains only (not FORWARD, since the
+// userspace router never forwards in the kernel), creates no netbird work
+// table, and removes its rules on Close.
+func TestInterfaceAllowerInputOnly(t *testing.T) {
+	if os.Geteuid() != 0 {
+		t.Skip("root required")
+	}
+
+	require.False(t, ipTableExists(t, getTableName()), "precondition: no stale netbird table")
+
+	conn := &nftables.Conn{}
+	extTable := conn.AddTable(&nftables.Table{Name: "nbtest_extchains", Family: nftables.TableFamilyINet})
+	inputChain := conn.AddChain(&nftables.Chain{
+		Name: "ext_input", Table: extTable,
+		Hooknum: nftables.ChainHookInput, Priority: nftables.ChainPriorityFilter, Type: nftables.ChainTypeFilter,
+	})
+	forwardChain := conn.AddChain(&nftables.Chain{
+		Name: "ext_forward", Table: extTable,
+		Hooknum: nftables.ChainHookForward, Priority: nftables.ChainPriorityFilter, Type: nftables.ChainTypeFilter,
+	})
+	require.NoError(t, conn.Flush(), "create external table and chains")
+	t.Cleanup(func() {
+		c := &nftables.Conn{}
+		c.DelTable(extTable)
+		_ = c.Flush()
+	})
+
+	allower, err := NewInterfaceAllower(ifaceMock, iface.DefaultMTU)
+	require.NoError(t, err, "create allower")
+	require.NoError(t, allower.Apply(), "apply")
+
+	require.True(t, chainHasUserData(t, extTable, inputChain, userDataAcceptInputRule),
+		"external INPUT chain should get the accept rule")
+	require.Len(t, listRules(t, extTable, forwardChain), 0,
+		"external FORWARD chain must not be opened in userspace mode")
+	require.False(t, ipTableExists(t, getTableName()),
+		"allower must not create a netbird work table")
+
+	require.NoError(t, allower.Close(), "close")
+	require.False(t, chainHasUserData(t, extTable, inputChain, userDataAcceptInputRule),
+		"accept rule should be removed on close")
+}
+
+func listRules(t *testing.T, table *nftables.Table, chain *nftables.Chain) []*nftables.Rule {
+	t.Helper()
+	c := &nftables.Conn{}
+	rules, err := c.GetRules(table, chain)
+	require.NoError(t, err)
+	return rules
+}
+
+func chainHasUserData(t *testing.T, table *nftables.Table, chain *nftables.Chain, ud string) bool {
+	for _, r := range listRules(t, table, chain) {
+		if bytes.Equal(r.UserData, []byte(ud)) {
+			return true
+		}
+	}
+	return false
+}
+
+func ipTableExists(t *testing.T, name string) bool {
+	t.Helper()
+	c := &nftables.Conn{}
+	for _, fam := range []nftables.TableFamily{nftables.TableFamilyIPv4, nftables.TableFamilyIPv6} {
+		tbls, err := c.ListTablesOfFamily(fam)
+		require.NoError(t, err)
+		for _, tb := range tbls {
+			if tb.Name == name {
+				return true
+			}
+		}
+	}
+	return false
+}

client/firewall/nftables/interface_allower_linux.go

@@ -0,0 +1,114 @@
+package nftables
+
+import (
+	"fmt"
+
+	"github.com/google/nftables"
+	"github.com/hashicorp/go-multierror"
+
+	nberrors "github.com/netbirdio/netbird/client/errors"
+)
+
+// InterfaceAllower opens the NetBird interface in the kernel's filter table and
+// external chains and keeps them reconciled via a netlink monitor, so the host
+// firewall doesn't drop traffic the NetBird firewall handles. It is used by the
+// userspace firewall, where routing happens in the forwarder, so only INPUT is
+// opened (the userspace router never forwards in the kernel).
+//
+// It owns its own families/connection and never creates a netbird work table.
+// firewalld trust is handled by the caller, not here. Its operations are serial
+// (Apply before the monitor starts; reconciles run on the single monitor
+// goroutine; Close stops the monitor before removing), so it needs no locking.
+//
+// TODO: this opens nftables and the iptables-nft filter table (detected via
+// nft), but not a legacy-iptables ruleset running in parallel with nftables.
+// Such a host would keep its legacy filter chains closed for the interface.
+type InterfaceAllower struct {
+	family4    *family
+	family6    *family
+	extMonitor *externalChainMonitor
+}
+
+// NewInterfaceAllower builds an allower for the given interface. It returns an
+// error when nftables is unavailable (e.g. an iptables-legacy host), so the
+// caller can fall back to firewalld trust.
+func NewInterfaceAllower(wgIface iFaceMapper, mtu uint16) (*InterfaceAllower, error) {
+	tableName := getTableName()
+
+	family4, err := newFamily(&nftables.Table{Name: tableName, Family: nftables.TableFamilyIPv4}, wgIface, mtu)
+	if err != nil {
+		return nil, fmt.Errorf("create family: %w", err)
+	}
+
+	// Probe nftables availability before committing to this backend.
+	if _, err := family4.conn.ListChainsOfTableFamily(nftables.TableFamilyINet); err != nil {
+		return nil, fmt.Errorf("nftables not available: %w", err)
+	}
+
+	a := &InterfaceAllower{family4: family4}
+
+	if wgIface.Address().HasIPv6() {
+		family6, err := newFamily(&nftables.Table{Name: tableName, Family: nftables.TableFamilyIPv6}, wgIface, mtu)
+		if err != nil {
+			return nil, fmt.Errorf("create v6 family: %w", err)
+		}
+		a.family6 = family6
+	}
+
+	a.extMonitor = newExternalChainMonitor(a)
+	return a, nil
+}
+
+// Apply opens the interface (INPUT only) in the foreign filter chains and starts
+// reconciling them on nftables changes.
+func (a *InterfaceAllower) Apply() error {
+	var merr *multierror.Error
+	for _, f := range a.families() {
+		// Remove any stale accepts first so a prior unclean exit (e.g. SIGKILL,
+		// where Close never ran) is recovered deterministically rather than
+		// accumulating duplicate rules on the iptables filter table.
+		if err := f.removeAcceptFilterRules(); err != nil {
+			merr = multierror.Append(merr, fmt.Errorf("clean stale accept rules: %w", err))
+		}
+		if err := f.openInterface(false); err != nil {
+			merr = multierror.Append(merr, err)
+		}
+	}
+
+	a.extMonitor.start()
+	return nberrors.FormatErrorOrNil(merr)
+}
+
+// families returns the configured address families (v4, and v6 when present).
+func (a *InterfaceAllower) families() []*family {
+	families := []*family{a.family4}
+	if a.family6 != nil {
+		families = append(families, a.family6)
+	}
+	return families
+}
+
+// reconcileExternalChains re-applies the INPUT accepts to external chains. It
+// implements externalChainReconciler for the monitor.
+func (a *InterfaceAllower) reconcileExternalChains() error {
+	var merr *multierror.Error
+	for _, f := range a.families() {
+		if err := f.acceptExternalChainsRules(false); err != nil {
+			merr = multierror.Append(merr, err)
+		}
+	}
+	return nberrors.FormatErrorOrNil(merr)
+}
+
+// Close stops the monitor and removes the accept rules.
+func (a *InterfaceAllower) Close() error {
+	a.extMonitor.stop()
+
+	var merr *multierror.Error
+	for _, f := range a.families() {
+		if err := f.removeAcceptFilterRules(); err != nil {
+			merr = multierror.Append(merr, err)
+		}
+	}
+	return nberrors.FormatErrorOrNil(merr)
+}

client/firewall/nftables/manager_linux.go

@@ -15,7 +15,6 @@ import (
 	"golang.org/x/sys/unix"
 
 	nberrors "github.com/netbirdio/netbird/client/errors"
-	"github.com/netbirdio/netbird/client/firewall/firewalld"
 	firewall "github.com/netbirdio/netbird/client/firewall/manager"
 	"github.com/netbirdio/netbird/client/iface/wgaddr"
 	"github.com/netbirdio/netbird/client/internal/statemanager"
@@ -140,19 +139,20 @@ func (m *Manager) Init(stateManager *statemanager.Manager) error {
 
 // reconcileExternalChains re-applies passthrough accept rules to external
 // filter chains for both IPv4 and IPv6 routers. Called by the monitor when
-// tables or chains appear (e.g. after firewalld reloads).
+// tables or chains appear (e.g. after firewalld reloads). Kernel routing opens
+// both INPUT and FORWARD.
 func (m *Manager) reconcileExternalChains() error {
 	m.mutex.Lock()
 	defer m.mutex.Unlock()
 
 	var merr *multierror.Error
 	if m.family4 != nil {
-		if err := m.family4.acceptExternalChainsRules(); err != nil {
+		if err := m.family4.acceptExternalChainsRules(true); err != nil {
 			merr = multierror.Append(merr, fmt.Errorf("v4: %w", err))
 		}
 	}
 	if m.hasIPv6() {
-		if err := m.family6.acceptExternalChainsRules(); err != nil {
+		if err := m.family6.acceptExternalChainsRules(true); err != nil {
 			merr = multierror.Append(merr, fmt.Errorf("v6: %w", err))
 		}
 	}
@@ -370,39 +370,6 @@ func (m *Manager) RemoveNatRule(pair firewall.RouterPair) error {
 	return nberrors.FormatErrorOrNil(merr)
 }
 
-// AllowNetbird allows netbird interface traffic.
-// This is called when USPFilter wraps the native firewall, adding blanket accept
-// rules so that packet filtering is handled in userspace instead of by netfilter.
-//
-// TODO: In USP mode this only adds ACCEPT to the netbird table's own chains,
-// which doesn't override DROP rules in external tables (e.g. firewalld).
-// Should add passthrough rules to external chains (like the native mode router's
-// addExternalChainsRules does) for both the netbird table family and inet tables.
-// The netbird table itself is fine (routing chains already exist there), but
-// non-netbird tables with INPUT/FORWARD hooks can still DROP our WG traffic.
-func (m *Manager) AllowNetbird() error {
-	m.mutex.Lock()
-	defer m.mutex.Unlock()
-
-	if err := m.family4.createDefaultAllowRules(); err != nil {
-		return fmt.Errorf("create default allow rules: %w", err)
-	}
-	if m.hasIPv6() {
-		if err := m.family6.createDefaultAllowRules(); err != nil {
-			return fmt.Errorf("create v6 default allow rules: %w", err)
-		}
-	}
-	if err := m.rConn.Flush(); err != nil {
-		return fmt.Errorf("flush allow input netbird rules: %w", err)
-	}
-
-	if err := firewalld.TrustInterface(m.wgIface.Name()); err != nil {
-		log.Warnf("failed to trust interface in firewalld: %v", err)
-	}
-
-	return nil
-}
-
 // SetLegacyManagement sets the route manager to use legacy management
 func (m *Manager) SetLegacyManagement(isLegacy bool) error {
 	if err := firewall.SetLegacyManagement(m.family4, isLegacy); err != nil {