[client] Close userspace firewall properly (#3426)

This commit is contained in:
Viktor Liu
2025-03-04 11:19:42 +01:00
committed by Pedro Costa
parent e41acdb9ac
commit 85aea0a030
16 changed files with 43 additions and 43 deletions

View File

@@ -167,7 +167,7 @@ func (m *Manager) SetLegacyManagement(isLegacy bool) error {
}
// Reset firewall to the default state
func (m *Manager) Reset(stateManager *statemanager.Manager) error {
func (m *Manager) Close(stateManager *statemanager.Manager) error {
m.mutex.Lock()
defer m.mutex.Unlock()

View File

@@ -62,7 +62,7 @@ func TestIptablesManager(t *testing.T) {
time.Sleep(time.Second)
defer func() {
err := manager.Reset(nil)
err := manager.Close(nil)
require.NoError(t, err, "clear the manager state")
time.Sleep(time.Second)
@@ -100,14 +100,14 @@ func TestIptablesManager(t *testing.T) {
_, err = manager.AddPeerFiltering(nil, ip, "udp", nil, port, fw.ActionAccept, "")
require.NoError(t, err, "failed to add rule")
err = manager.Reset(nil)
err = manager.Close(nil)
require.NoError(t, err, "failed to reset")
ok, err := ipv4Client.ChainExists("filter", chainNameInputRules)
require.NoError(t, err, "failed check chain exists")
if ok {
require.NoErrorf(t, err, "chain '%v' still exists after Reset", chainNameInputRules)
require.NoErrorf(t, err, "chain '%v' still exists after Close", chainNameInputRules)
}
})
}
@@ -136,7 +136,7 @@ func TestIptablesManagerIPSet(t *testing.T) {
time.Sleep(time.Second)
defer func() {
err := manager.Reset(nil)
err := manager.Close(nil)
require.NoError(t, err, "clear the manager state")
time.Sleep(time.Second)
@@ -166,7 +166,7 @@ func TestIptablesManagerIPSet(t *testing.T) {
})
t.Run("reset check", func(t *testing.T) {
err = manager.Reset(nil)
err = manager.Close(nil)
require.NoError(t, err, "failed to reset")
})
}
@@ -204,7 +204,7 @@ func TestIptablesCreatePerformance(t *testing.T) {
time.Sleep(time.Second)
defer func() {
err := manager.Reset(nil)
err := manager.Close(nil)
require.NoError(t, err, "clear the manager state")
time.Sleep(time.Second)

View File

@@ -62,7 +62,7 @@ func (s *ShutdownState) Cleanup() error {
ipt.aclMgr.ipsetStore = s.ACLIPsetStore
}
if err := ipt.Reset(nil); err != nil {
if err := ipt.Close(nil); err != nil {
return fmt.Errorf("reset iptables manager: %w", err)
}

View File

@@ -102,8 +102,8 @@ type Manager interface {
// SetLegacyManagement sets the legacy management mode
SetLegacyManagement(legacy bool) error
// Reset firewall to the default state
Reset(stateManager *statemanager.Manager) error
// Close closes the firewall manager
Close(stateManager *statemanager.Manager) error
// Flush the changes to firewall controller
Flush() error

View File

@@ -87,7 +87,7 @@ func (m *Manager) Init(stateManager *statemanager.Manager) error {
// We only need to record minimal interface state for potential recreation.
// Unlike iptables, which requires tracking individual rules, nftables maintains
// a known state (our netbird table plus a few static rules). This allows for easy
// cleanup using Reset() without needing to store specific rules.
// cleanup using Close() without needing to store specific rules.
if err := stateManager.UpdateState(&ShutdownState{
InterfaceState: &InterfaceState{
NameStr: m.wgIface.Name(),
@@ -243,7 +243,7 @@ func (m *Manager) SetLegacyManagement(isLegacy bool) error {
}
// Reset firewall to the default state
func (m *Manager) Reset(stateManager *statemanager.Manager) error {
func (m *Manager) Close(stateManager *statemanager.Manager) error {
m.mutex.Lock()
defer m.mutex.Unlock()

View File

@@ -65,7 +65,7 @@ func TestNftablesManager(t *testing.T) {
time.Sleep(time.Second * 3)
defer func() {
err = manager.Reset(nil)
err = manager.Close(nil)
require.NoError(t, err, "failed to reset")
time.Sleep(time.Second)
}()
@@ -162,7 +162,7 @@ func TestNftablesManager(t *testing.T) {
// established rule remains
require.Len(t, rules, 1, "expected 1 rules after deletion")
err = manager.Reset(nil)
err = manager.Close(nil)
require.NoError(t, err, "failed to reset")
}
@@ -191,7 +191,7 @@ func TestNFtablesCreatePerformance(t *testing.T) {
time.Sleep(time.Second * 3)
defer func() {
if err := manager.Reset(nil); err != nil {
if err := manager.Close(nil); err != nil {
t.Errorf("clear the manager state: %v", err)
}
time.Sleep(time.Second)
@@ -274,7 +274,7 @@ func TestNftablesManagerCompatibilityWithIptables(t *testing.T) {
require.NoError(t, manager.Init(nil))
t.Cleanup(func() {
err := manager.Reset(nil)
err := manager.Close(nil)
require.NoError(t, err, "failed to reset manager state")
// Verify iptables output after reset

View File

@@ -38,7 +38,7 @@ func TestNftablesManager_AddNatRule(t *testing.T) {
// need fw manager to init both acl mgr and router for all chains to be present
manager, err := Create(ifaceMock)
t.Cleanup(func() {
require.NoError(t, manager.Reset(nil))
require.NoError(t, manager.Close(nil))
})
require.NoError(t, err)
require.NoError(t, manager.Init(nil))
@@ -127,7 +127,7 @@ func TestNftablesManager_RemoveNatRule(t *testing.T) {
t.Run(testCase.Name, func(t *testing.T) {
manager, err := Create(ifaceMock)
t.Cleanup(func() {
require.NoError(t, manager.Reset(nil))
require.NoError(t, manager.Close(nil))
})
require.NoError(t, err)
require.NoError(t, manager.Init(nil))

View File

@@ -39,7 +39,7 @@ func (s *ShutdownState) Cleanup() error {
return fmt.Errorf("create nftables manager: %w", err)
}
if err := nft.Reset(nil); err != nil {
if err := nft.Close(nil); err != nil {
return fmt.Errorf("reset nftables manager: %w", err)
}

View File

@@ -13,7 +13,7 @@ import (
)
// Reset firewall to the default state
func (m *Manager) Reset(stateManager *statemanager.Manager) error {
func (m *Manager) Close(stateManager *statemanager.Manager) error {
m.mutex.Lock()
defer m.mutex.Unlock()
@@ -48,7 +48,7 @@ func (m *Manager) Reset(stateManager *statemanager.Manager) error {
}
if m.nativeFirewall != nil {
return m.nativeFirewall.Reset(stateManager)
return m.nativeFirewall.Close(stateManager)
}
return nil
}

View File

@@ -21,8 +21,8 @@ const (
firewallRuleName = "Netbird"
)
// Reset firewall to the default state
func (m *Manager) Reset(*statemanager.Manager) error {
// Close closes the firewall manager
func (m *Manager) Close(*statemanager.Manager) error {
m.mutex.Lock()
defer m.mutex.Unlock()

View File

@@ -171,7 +171,7 @@ func BenchmarkCoreFiltering(b *testing.B) {
SetFilterFunc: func(device.PacketFilter) error { return nil },
}, false, flowLogger)
defer b.Cleanup(func() {
require.NoError(b, manager.Reset(nil))
require.NoError(b, manager.Close(nil))
})
manager.wgNetwork = &net.IPNet{
@@ -216,7 +216,7 @@ func BenchmarkStateScaling(b *testing.B) {
SetFilterFunc: func(device.PacketFilter) error { return nil },
}, false, flowLogger)
b.Cleanup(func() {
require.NoError(b, manager.Reset(nil))
require.NoError(b, manager.Close(nil))
})
manager.wgNetwork = &net.IPNet{
@@ -264,7 +264,7 @@ func BenchmarkEstablishmentOverhead(b *testing.B) {
SetFilterFunc: func(device.PacketFilter) error { return nil },
}, false, flowLogger)
b.Cleanup(func() {
require.NoError(b, manager.Reset(nil))
require.NoError(b, manager.Close(nil))
})
manager.wgNetwork = &net.IPNet{
@@ -463,7 +463,7 @@ func BenchmarkRoutedNetworkReturn(b *testing.B) {
SetFilterFunc: func(device.PacketFilter) error { return nil },
}, false, flowLogger)
b.Cleanup(func() {
require.NoError(b, manager.Reset(nil))
require.NoError(b, manager.Close(nil))
})
// Setup scenario
@@ -590,7 +590,7 @@ func BenchmarkLongLivedConnections(b *testing.B) {
SetFilterFunc: func(device.PacketFilter) error { return nil },
}, false, flowLogger)
defer b.Cleanup(func() {
require.NoError(b, manager.Reset(nil))
require.NoError(b, manager.Close(nil))
})
manager.SetNetwork(&net.IPNet{
@@ -678,7 +678,7 @@ func BenchmarkShortLivedConnections(b *testing.B) {
SetFilterFunc: func(device.PacketFilter) error { return nil },
}, false, flowLogger)
defer b.Cleanup(func() {
require.NoError(b, manager.Reset(nil))
require.NoError(b, manager.Close(nil))
})
manager.SetNetwork(&net.IPNet{
@@ -794,7 +794,7 @@ func BenchmarkParallelLongLivedConnections(b *testing.B) {
SetFilterFunc: func(device.PacketFilter) error { return nil },
}, false, flowLogger)
defer b.Cleanup(func() {
require.NoError(b, manager.Reset(nil))
require.NoError(b, manager.Close(nil))
})
manager.SetNetwork(&net.IPNet{
@@ -879,7 +879,7 @@ func BenchmarkParallelShortLivedConnections(b *testing.B) {
SetFilterFunc: func(device.PacketFilter) error { return nil },
}, false, flowLogger)
defer b.Cleanup(func() {
require.NoError(b, manager.Reset(nil))
require.NoError(b, manager.Close(nil))
})
manager.SetNetwork(&net.IPNet{

View File

@@ -39,7 +39,7 @@ func TestPeerACLFiltering(t *testing.T) {
require.NotNil(t, manager)
t.Cleanup(func() {
require.NoError(t, manager.Reset(nil))
require.NoError(t, manager.Close(nil))
})
manager.wgNetwork = wgNet
@@ -310,7 +310,7 @@ func setupRoutedManager(tb testing.TB, network string) *Manager {
require.False(tb, manager.nativeRouter)
tb.Cleanup(func() {
require.NoError(tb, manager.Reset(nil))
require.NoError(tb, manager.Close(nil))
})
return manager

View File

@@ -255,7 +255,7 @@ func TestManagerReset(t *testing.T) {
return
}
err = m.Reset(nil)
err = m.Close(nil)
if err != nil {
t.Errorf("failed to reset Manager: %v", err)
return
@@ -333,7 +333,7 @@ func TestNotMatchByIP(t *testing.T) {
return
}
if err = m.Reset(nil); err != nil {
if err = m.Close(nil); err != nil {
t.Errorf("failed to reset Manager: %v", err)
return
}
@@ -352,7 +352,7 @@ func TestRemovePacketHook(t *testing.T) {
t.Fatalf("Failed to create Manager: %s", err)
}
defer func() {
require.NoError(t, manager.Reset(nil))
require.NoError(t, manager.Close(nil))
}()
// Add a UDP packet hook
@@ -403,7 +403,7 @@ func TestProcessOutgoingHooks(t *testing.T) {
manager.udpTracker.Close()
manager.udpTracker = conntrack.NewUDPTracker(100*time.Millisecond, logger, flowLogger)
defer func() {
require.NoError(t, manager.Reset(nil))
require.NoError(t, manager.Close(nil))
}()
manager.decoders = sync.Pool{
@@ -484,7 +484,7 @@ func TestUSPFilterCreatePerformance(t *testing.T) {
time.Sleep(time.Second)
defer func() {
if err := manager.Reset(nil); err != nil {
if err := manager.Close(nil); err != nil {
t.Errorf("clear the manager state: %v", err)
}
time.Sleep(time.Second)
@@ -530,7 +530,7 @@ func TestStatefulFirewall_UDPTracking(t *testing.T) {
},
}
defer func() {
require.NoError(t, manager.Reset(nil))
require.NoError(t, manager.Close(nil))
}()
// Set up packet parameters