Fix routes allow acl rule (#940)

Modify rules in iptables and nftables to accept all traffic not from netbird network but routed through it.

client/firewall/iptables/manager_linux.go

@@ -11,6 +11,7 @@ import (
 	log "github.com/sirupsen/logrus"
 
 	fw "github.com/netbirdio/netbird/client/firewall"
+	"github.com/netbirdio/netbird/iface"
 )
 
 const (
@@ -21,12 +22,6 @@ const (
 	ChainOutputFilterName = "NETBIRD-ACL-OUTPUT"
 )
 
-// jumpNetbirdInputDefaultRule always added by manager to the input chain for all trafic from the Netbird interface
-var jumpNetbirdInputDefaultRule = []string{"-j", ChainInputFilterName}
-
-// jumpNetbirdOutputDefaultRule always added by manager to the output chain for all trafic from the Netbird interface
-var jumpNetbirdOutputDefaultRule = []string{"-j", ChainOutputFilterName}
-
 // dropAllDefaultRule in the Netbird chain
 var dropAllDefaultRule = []string{"-j", "DROP"}
 
@@ -37,13 +32,25 @@ type Manager struct {
 	ipv4Client *iptables.IPTables
 	ipv6Client *iptables.IPTables
 
-	wgIfaceName string
+	inputDefaultRuleSpecs  []string
+	outputDefaultRuleSpecs []string
+	wgIface                iFaceMapper
+}
+
+// iFaceMapper defines subset methods of interface required for manager
+type iFaceMapper interface {
+	Name() string
+	Address() iface.WGAddress
 }
 
 // Create iptables firewall manager
-func Create(wgIfaceName string) (*Manager, error) {
+func Create(wgIface iFaceMapper) (*Manager, error) {
 	m := &Manager{
-		wgIfaceName: wgIfaceName,
+		wgIface: wgIface,
+		inputDefaultRuleSpecs: []string{
+			"-i", wgIface.Name(), "-j", ChainInputFilterName, "-s", wgIface.Address().String()},
+		outputDefaultRuleSpecs: []string{
+			"-o", wgIface.Name(), "-j", ChainOutputFilterName, "-d", wgIface.Address().String()},
 	}
 
 	// init clients for booth ipv4 and ipv6
@@ -193,11 +200,10 @@ func (m *Manager) reset(client *iptables.IPTables, table string) error {
 		return fmt.Errorf("failed to check if input chain exists: %w", err)
 	}
 	if ok {
-		specs := append([]string{"-i", m.wgIfaceName}, jumpNetbirdInputDefaultRule...)
+		if ok, err := client.Exists("filter", "INPUT", m.inputDefaultRuleSpecs...); err != nil {
-		if ok, err := client.Exists("filter", "INPUT", specs...); err != nil {
 			return err
 		} else if ok {
-			if err := client.Delete("filter", "INPUT", specs...); err != nil {
+			if err := client.Delete("filter", "INPUT", m.inputDefaultRuleSpecs...); err != nil {
 				log.WithError(err).Errorf("failed to delete default input rule: %v", err)
 			}
 		}
@@ -208,11 +214,10 @@ func (m *Manager) reset(client *iptables.IPTables, table string) error {
 		return fmt.Errorf("failed to check if output chain exists: %w", err)
 	}
 	if ok {
-		specs := append([]string{"-o", m.wgIfaceName}, jumpNetbirdOutputDefaultRule...)
+		if ok, err := client.Exists("filter", "OUTPUT", m.outputDefaultRuleSpecs...); err != nil {
-		if ok, err := client.Exists("filter", "OUTPUT", specs...); err != nil {
 			return err
 		} else if ok {
-			if err := client.Delete("filter", "OUTPUT", specs...); err != nil {
+			if err := client.Delete("filter", "OUTPUT", m.outputDefaultRuleSpecs...); err != nil {
 				log.WithError(err).Errorf("failed to delete default output rule: %v", err)
 			}
 		}
@@ -296,8 +301,7 @@ func (m *Manager) client(ip net.IP) (*iptables.IPTables, error) {
 			return nil, fmt.Errorf("failed to create default drop all in netbird input chain: %w", err)
 		}
 
-		specs := append([]string{"-i", m.wgIfaceName}, jumpNetbirdInputDefaultRule...)
+		if err := client.AppendUnique("filter", "INPUT", m.inputDefaultRuleSpecs...); err != nil {
-		if err := client.AppendUnique("filter", "INPUT", specs...); err != nil {
 			return nil, fmt.Errorf("failed to create input chain jump rule: %w", err)
 		}
 
@@ -317,8 +321,7 @@ func (m *Manager) client(ip net.IP) (*iptables.IPTables, error) {
 			return nil, fmt.Errorf("failed to create default drop all in netbird output chain: %w", err)
 		}
 
-		specs := append([]string{"-o", m.wgIfaceName}, jumpNetbirdOutputDefaultRule...)
+		if err := client.AppendUnique("filter", "OUTPUT", m.outputDefaultRuleSpecs...); err != nil {
-		if err := client.AppendUnique("filter", "OUTPUT", specs...); err != nil {
 			return nil, fmt.Errorf("failed to create output chain jump rule: %w", err)
 		}
 	}

client/firewall/iptables/manager_linux_test.go

@@ -10,14 +10,50 @@ import (
 	"github.com/stretchr/testify/require"
 
 	fw "github.com/netbirdio/netbird/client/firewall"
+	"github.com/netbirdio/netbird/iface"
 )
 
+// iFaceMapper defines subset methods of interface required for manager
+type iFaceMock struct {
+	NameFunc    func() string
+	AddressFunc func() iface.WGAddress
+}
+
+func (i *iFaceMock) Name() string {
+	if i.NameFunc != nil {
+		return i.NameFunc()
+	}
+	panic("NameFunc is not set")
+}
+
+func (i *iFaceMock) Address() iface.WGAddress {
+	if i.AddressFunc != nil {
+		return i.AddressFunc()
+	}
+	panic("AddressFunc is not set")
+}
+
 func TestIptablesManager(t *testing.T) {
 	ipv4Client, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
 	require.NoError(t, err)
 
+	mock := &iFaceMock{
+		NameFunc: func() string {
+			return "lo"
+		},
+		AddressFunc: func() iface.WGAddress {
+			return iface.WGAddress{
+				IP: net.ParseIP("10.20.0.1"),
+				Network: &net.IPNet{
+					IP:   net.ParseIP("10.20.0.0"),
+					Mask: net.IPv4Mask(255, 255, 255, 0),
+				},
+			}
+		},
+	}
+
 	// just check on the local interface
-	manager, err := Create("lo")
+	manager, err := Create(mock)
 	require.NoError(t, err)
 	time.Sleep(time.Second)
 
@@ -94,10 +130,25 @@ func checkRuleSpecs(t *testing.T, ipv4Client *iptables.IPTables, chainName strin
 }
 
 func TestIptablesCreatePerformance(t *testing.T) {
+	mock := &iFaceMock{
+		NameFunc: func() string {
+			return "lo"
+		},
+		AddressFunc: func() iface.WGAddress {
+			return iface.WGAddress{
+				IP: net.ParseIP("10.20.0.1"),
+				Network: &net.IPNet{
+					IP:   net.ParseIP("10.20.0.0"),
+					Mask: net.IPv4Mask(255, 255, 255, 0),
+				},
+			}
+		},
+	}
+
 	for _, testMax := range []int{10, 20, 30, 40, 50, 60, 70, 80, 90, 100, 200, 300, 400, 500, 600, 700, 800, 900, 1000} {
 		t.Run(fmt.Sprintf("Testing %d rules", testMax), func(t *testing.T) {
 			// just check on the local interface
-			manager, err := Create("lo")
+			manager, err := Create(mock)
 			require.NoError(t, err)
 			time.Sleep(time.Second)
 

client/firewall/nftables/manager_linux.go

@@ -15,6 +15,7 @@ import (
 	"golang.org/x/sys/unix"
 
 	fw "github.com/netbirdio/netbird/client/firewall"
+	"github.com/netbirdio/netbird/iface"
 )
 
 const (
@@ -42,14 +43,20 @@ type Manager struct {
 	filterInputChainIPv6  *nftables.Chain
 	filterOutputChainIPv6 *nftables.Chain
 
-	wgIfaceName string
+	wgIface iFaceMapper
+}
+
+// iFaceMapper defines subset methods of interface required for manager
+type iFaceMapper interface {
+	Name() string
+	Address() iface.WGAddress
 }
 
 // Create nftables firewall manager
-func Create(wgIfaceName string) (*Manager, error) {
+func Create(wgIface iFaceMapper) (*Manager, error) {
 	m := &Manager{
 		conn:    &nftables.Conn{},
-		wgIfaceName: wgIfaceName,
+		wgIface: wgIface,
 	}
 
 	if err := m.Reset(); err != nil {
@@ -109,7 +116,7 @@ func (m *Manager) AddFiltering(
 		&expr.Cmp{
 			Op:       expr.CmpOpEq,
 			Register: 1,
-			Data:     ifname(m.wgIfaceName),
+			Data:     ifname(m.wgIface.Name()),
 		},
 	}
 
@@ -358,15 +365,82 @@ func (m *Manager) createChainIfNotExists(
 	chain = m.conn.AddChain(chain)
 
 	ifaceKey := expr.MetaKeyIIFNAME
+	shiftDSTAddr := 0
 	if name == FilterOutputChainName {
 		ifaceKey = expr.MetaKeyOIFNAME
+		shiftDSTAddr = 1
 	}
+
 	expressions := []expr.Any{
 		&expr.Meta{Key: ifaceKey, Register: 1},
 		&expr.Cmp{
 			Op:       expr.CmpOpEq,
 			Register: 1,
-			Data:     ifname(m.wgIfaceName),
+			Data:     ifname(m.wgIface.Name()),
+		},
+	}
+
+	mask, _ := netip.AddrFromSlice(m.wgIface.Address().Network.Mask)
+	if m.wgIface.Address().IP.To4() == nil {
+		ip, _ := netip.AddrFromSlice(m.wgIface.Address().Network.IP.To16())
+		expressions = append(expressions,
+			&expr.Payload{
+				DestRegister: 2,
+				Base:         expr.PayloadBaseNetworkHeader,
+				Offset:       uint32(8 + (16 * shiftDSTAddr)),
+				Len:          16,
+			},
+			&expr.Bitwise{
+				SourceRegister: 2,
+				DestRegister:   2,
+				Len:            16,
+				Xor:            []byte{0x0, 0x0, 0x0, 0x0},
+				Mask:           mask.Unmap().AsSlice(),
+			},
+			&expr.Cmp{
+				Op:       expr.CmpOpNeq,
+				Register: 2,
+				Data:     ip.Unmap().AsSlice(),
+			},
+			&expr.Verdict{Kind: expr.VerdictAccept},
+		)
+	} else {
+		ip, _ := netip.AddrFromSlice(m.wgIface.Address().Network.IP.To4())
+		expressions = append(expressions,
+			&expr.Payload{
+				DestRegister: 2,
+				Base:         expr.PayloadBaseNetworkHeader,
+				Offset:       uint32(12 + (4 * shiftDSTAddr)),
+				Len:          4,
+			},
+			&expr.Bitwise{
+				SourceRegister: 2,
+				DestRegister:   2,
+				Len:            4,
+				Xor:            []byte{0x0, 0x0, 0x0, 0x0},
+				Mask:           m.wgIface.Address().Network.Mask,
+			},
+			&expr.Cmp{
+				Op:       expr.CmpOpNeq,
+				Register: 2,
+				Data:     ip.Unmap().AsSlice(),
+			},
+			&expr.Verdict{Kind: expr.VerdictAccept},
+		)
+	}
+
+	_ = m.conn.AddRule(&nftables.Rule{
+		Table: table,
+		Chain: chain,
+		Exprs: expressions,
+	})
+
+	expressions = []expr.Any{
+		&expr.Meta{Key: ifaceKey, Register: 1},
+		&expr.Cmp{
+			Op:       expr.CmpOpEq,
+			Register: 1,
+			Data:     ifname(m.wgIface.Name()),
 		},
 		&expr.Verdict{Kind: expr.VerdictDrop},
 	}
@@ -375,7 +449,6 @@ func (m *Manager) createChainIfNotExists(
 		Chain: chain,
 		Exprs: expressions,
 	})
-
 	if err := m.conn.Flush(); err != nil {
 		return nil, err
 	}

client/firewall/nftables/manager_linux_test.go

@@ -13,11 +13,47 @@ import (
 	"golang.org/x/sys/unix"
 
 	fw "github.com/netbirdio/netbird/client/firewall"
+	"github.com/netbirdio/netbird/iface"
 )
 
+// iFaceMapper defines subset methods of interface required for manager
+type iFaceMock struct {
+	NameFunc    func() string
+	AddressFunc func() iface.WGAddress
+}
+
+func (i *iFaceMock) Name() string {
+	if i.NameFunc != nil {
+		return i.NameFunc()
+	}
+	panic("NameFunc is not set")
+}
+
+func (i *iFaceMock) Address() iface.WGAddress {
+	if i.AddressFunc != nil {
+		return i.AddressFunc()
+	}
+	panic("AddressFunc is not set")
+}
+
 func TestNftablesManager(t *testing.T) {
+	mock := &iFaceMock{
+		NameFunc: func() string {
+			return "lo"
+		},
+		AddressFunc: func() iface.WGAddress {
+			return iface.WGAddress{
+				IP: net.ParseIP("100.96.0.1"),
+				Network: &net.IPNet{
+					IP:   net.ParseIP("100.96.0.0"),
+					Mask: net.IPv4Mask(255, 255, 255, 0),
+				},
+			}
+		},
+	}
+
 	// just check on the local interface
-	manager, err := Create("lo")
+	manager, err := Create(mock)
 	require.NoError(t, err)
 	time.Sleep(time.Second)
 
@@ -44,8 +80,11 @@ func TestNftablesManager(t *testing.T) {
 
 	rules, err := testClient.GetRules(manager.tableIPv4, manager.filterInputChainIPv4)
 	require.NoError(t, err, "failed to get rules")
-	// 1 regular rule and other "drop all rule" for the interface
+	// test expectations:
-	require.Len(t, rules, 2, "expected 1 rule")
+	// 1) regular rule
+	// 2) "accept extra routed traffic rule" for the interface
+	// 3) "drop all rule" for the interface
+	require.Len(t, rules, 3, "expected 3 rules")
 
 	ipToAdd, _ := netip.AddrFromSlice(ip)
 	add := ipToAdd.Unmap()
@@ -98,17 +137,35 @@ func TestNftablesManager(t *testing.T) {
 
 	rules, err = testClient.GetRules(manager.tableIPv4, manager.filterInputChainIPv4)
 	require.NoError(t, err, "failed to get rules")
-	require.Len(t, rules, 1, "expected 1 rules after deleteion")
+	// test expectations:
+	// 1) "accept extra routed traffic rule" for the interface
+	// 2) "drop all rule" for the interface
+	require.Len(t, rules, 2, "expected 2 rules after deleteion")
 
 	err = manager.Reset()
 	require.NoError(t, err, "failed to reset")
 }
 
 func TestNFtablesCreatePerformance(t *testing.T) {
+	mock := &iFaceMock{
+		NameFunc: func() string {
+			return "lo"
+		},
+		AddressFunc: func() iface.WGAddress {
+			return iface.WGAddress{
+				IP: net.ParseIP("100.96.0.1"),
+				Network: &net.IPNet{
+					IP:   net.ParseIP("100.96.0.0"),
+					Mask: net.IPv4Mask(255, 255, 255, 0),
+				},
+			}
+		},
+	}
+
 	for _, testMax := range []int{10, 20, 30, 40, 50, 60, 70, 80, 90, 100, 200, 300, 400, 500, 600, 700, 800, 900, 1000} {
 		t.Run(fmt.Sprintf("Testing %d rules", testMax), func(t *testing.T) {
 			// just check on the local interface
-			manager, err := Create("lo")
+			manager, err := Create(mock)
 			require.NoError(t, err)
 			time.Sleep(time.Second)
 

client/internal/acl/manager.go

@@ -14,9 +14,10 @@ import (
 	mgmProto "github.com/netbirdio/netbird/management/proto"
 )
 
-// iFaceMapper defines subset methods of interface required for manager
+// IFaceMapper defines subset methods of interface required for manager
-type iFaceMapper interface {
+type IFaceMapper interface {
 	Name() string
+	Address() iface.WGAddress
 	IsUserspaceBind() bool
 	SetFiltering(iface.PacketFilter) error
 }

client/internal/acl/manager_create.go

@@ -11,7 +11,7 @@ import (
 )
 
 // Create creates a firewall manager instance
-func Create(iface iFaceMapper) (manager *DefaultManager, err error) {
+func Create(iface IFaceMapper) (manager *DefaultManager, err error) {
 	if iface.IsUserspaceBind() {
 		// use userspace packet filtering firewall
 		fm, err := uspfilter.Create(iface)

client/internal/acl/manager_create_linux.go

@@ -10,7 +10,7 @@ import (
 )
 
 // Create creates a firewall manager instance for the Linux
-func Create(iface iFaceMapper) (manager *DefaultManager, err error) {
+func Create(iface IFaceMapper) (manager *DefaultManager, err error) {
 	var fm firewall.Manager
 	if iface.IsUserspaceBind() {
 		// use userspace packet filtering firewall
@@ -19,10 +19,10 @@ func Create(iface iFaceMapper) (manager *DefaultManager, err error) {
 			return nil, err
 		}
 	} else {
-		if fm, err = nftables.Create(iface.Name()); err != nil {
+		if fm, err = nftables.Create(iface); err != nil {
 			log.Debugf("failed to create nftables manager: %s", err)
 			// fallback to iptables
-			if fm, err = iptables.Create(iface.Name()); err != nil {
+			if fm, err = iptables.Create(iface); err != nil {
 				log.Errorf("failed to create iptables manager: %s", err)
 				return nil, err
 			}

client/internal/acl/mocks/README.md

@@ -0,0 +1,7 @@
+## Mocks
+
+To generate (or refresh) mocks from acl package please install [mockgen](https://github.com/golang/mock).
+Run this command from the `./client/internal/acl` folder to update iface mapper interface mock:
+```bash
+mockgen -destination mocks/iface_mapper.go -package mocks . IFaceMapper
+```

client/internal/acl/mocks/iface_mapper.go

@@ -34,6 +34,20 @@ func (m *MockIFaceMapper) EXPECT() *MockIFaceMapperMockRecorder {
 	return m.recorder
 }
 
+// Address mocks base method.
+func (m *MockIFaceMapper) Address() iface.WGAddress {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "Address")
+	ret0, _ := ret[0].(iface.WGAddress)
+	return ret0
+}
+
+// Address indicates an expected call of Address.
+func (mr *MockIFaceMapperMockRecorder) Address() *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Address", reflect.TypeOf((*MockIFaceMapper)(nil).Address))
+}
+
 // IsUserspaceBind mocks base method.
 func (m *MockIFaceMapper) IsUserspaceBind() bool {
 	m.ctrl.T.Helper()