Skip to content

Commit bcc5824

Browse files
lixmallixmal
authored
[client] Close userspace firewall properly (#3426)
1 parent af5796d commit bcc5824

16 files changed

+43
-51
lines changed

‎client/firewall/iptables/manager_linux.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ func (m *Manager) SetLegacyManagement(isLegacy bool) error {
166166
}
167167

168168
// Reset firewall to the default state
169-
func (m *Manager) Reset(stateManager *statemanager.Manager) error {
169+
func (m *Manager) Close(stateManager *statemanager.Manager) error {
170170
m.mutex.Lock()
171171
defer m.mutex.Unlock()
172172

‎client/firewall/iptables/manager_linux_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ func TestIptablesManager(t *testing.T) {
6262
time.Sleep(time.Second)
6363

6464
defer func() {
65-
err := manager.Reset(nil)
65+
err := manager.Close(nil)
6666
require.NoError(t, err, "clear the manager state")
6767

6868
time.Sleep(time.Second)
@@ -100,14 +100,14 @@ func TestIptablesManager(t *testing.T) {
100100
_, err = manager.AddPeerFiltering(ip, "udp", nil, port, fw.ActionAccept, "", "accept Fake DNS traffic")
101101
require.NoError(t, err, "failed to add rule")
102102

103-
err = manager.Reset(nil)
103+
err = manager.Close(nil)
104104
require.NoError(t, err, "failed to reset")
105105

106106
ok, err := ipv4Client.ChainExists("filter", chainNameInputRules)
107107
require.NoError(t, err, "failed check chain exists")
108108

109109
if ok {
110-
require.NoErrorf(t, err, "chain '%v' still exists after Reset", chainNameInputRules)
110+
require.NoErrorf(t, err, "chain '%v' still exists after Close", chainNameInputRules)
111111
}
112112
})
113113
}
@@ -136,7 +136,7 @@ func TestIptablesManagerIPSet(t *testing.T) {
136136
time.Sleep(time.Second)
137137

138138
defer func() {
139-
err := manager.Reset(nil)
139+
err := manager.Close(nil)
140140
require.NoError(t, err, "clear the manager state")
141141

142142
time.Sleep(time.Second)
@@ -166,7 +166,7 @@ func TestIptablesManagerIPSet(t *testing.T) {
166166
})
167167

168168
t.Run("reset check", func(t *testing.T) {
169-
err = manager.Reset(nil)
169+
err = manager.Close(nil)
170170
require.NoError(t, err, "failed to reset")
171171
})
172172
}
@@ -204,7 +204,7 @@ func TestIptablesCreatePerformance(t *testing.T) {
204204
time.Sleep(time.Second)
205205

206206
defer func() {
207-
err := manager.Reset(nil)
207+
err := manager.Close(nil)
208208
require.NoError(t, err, "clear the manager state")
209209

210210
time.Sleep(time.Second)

‎client/firewall/iptables/state_linux.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ func (s *ShutdownState) Cleanup() error {
6262
ipt.aclMgr.ipsetStore = s.ACLIPsetStore
6363
}
6464

65-
if err := ipt.Reset(nil); err != nil {
65+
if err := ipt.Close(nil); err != nil {
6666
return fmt.Errorf("reset iptables manager: %w", err)
6767
}
6868

‎client/firewall/manager/firewall.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,8 @@ type Manager interface {
9494
// SetLegacyManagement sets the legacy management mode
9595
SetLegacyManagement(legacy bool) error
9696

97-
// Reset firewall to the default state
98-
Reset(stateManager *statemanager.Manager) error
97+
// Close closes the firewall manager
98+
Close(stateManager *statemanager.Manager) error
9999

100100
// Flush the changes to firewall controller
101101
Flush() error

‎client/firewall/nftables/manager_linux.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ func (m *Manager) Init(stateManager *statemanager.Manager) error {
8787
// We only need to record minimal interface state for potential recreation.
8888
// Unlike iptables, which requires tracking individual rules, nftables maintains
8989
// a known state (our netbird table plus a few static rules). This allows for easy
90-
// cleanup using Reset() without needing to store specific rules.
90+
// cleanup using Close() without needing to store specific rules.
9191
if err := stateManager.UpdateState(&ShutdownState{
9292
InterfaceState: &InterfaceState{
9393
NameStr: m.wgIface.Name(),
@@ -242,7 +242,7 @@ func (m *Manager) SetLegacyManagement(isLegacy bool) error {
242242
}
243243

244244
// Reset firewall to the default state
245-
func (m *Manager) Reset(stateManager *statemanager.Manager) error {
245+
func (m *Manager) Close(stateManager *statemanager.Manager) error {
246246
m.mutex.Lock()
247247
defer m.mutex.Unlock()
248248

‎client/firewall/nftables/manager_linux_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func TestNftablesManager(t *testing.T) {
6565
time.Sleep(time.Second * 3)
6666

6767
defer func() {
68-
err = manager.Reset(nil)
68+
err = manager.Close(nil)
6969
require.NoError(t, err, "failed to reset")
7070
time.Sleep(time.Second)
7171
}()
@@ -162,7 +162,7 @@ func TestNftablesManager(t *testing.T) {
162162
// established rule remains
163163
require.Len(t, rules, 1, "expected 1 rules after deletion")
164164

165-
err = manager.Reset(nil)
165+
err = manager.Close(nil)
166166
require.NoError(t, err, "failed to reset")
167167
}
168168

@@ -191,7 +191,7 @@ func TestNFtablesCreatePerformance(t *testing.T) {
191191
time.Sleep(time.Second * 3)
192192

193193
defer func() {
194-
if err := manager.Reset(nil); err != nil {
194+
if err := manager.Close(nil); err != nil {
195195
t.Errorf("clear the manager state: %v", err)
196196
}
197197
time.Sleep(time.Second)
@@ -274,7 +274,7 @@ func TestNftablesManagerCompatibilityWithIptables(t *testing.T) {
274274
require.NoError(t, manager.Init(nil))
275275

276276
t.Cleanup(func() {
277-
err := manager.Reset(nil)
277+
err := manager.Close(nil)
278278
require.NoError(t, err, "failed to reset manager state")
279279

280280
// Verify iptables output after reset

‎client/firewall/nftables/router_linux_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ func TestNftablesManager_AddNatRule(t *testing.T) {
3838
// need fw manager to init both acl mgr and router for all chains to be present
3939
manager, err := Create(ifaceMock)
4040
t.Cleanup(func() {
41-
require.NoError(t, manager.Reset(nil))
41+
require.NoError(t, manager.Close(nil))
4242
})
4343
require.NoError(t, err)
4444
require.NoError(t, manager.Init(nil))
@@ -127,7 +127,7 @@ func TestNftablesManager_RemoveNatRule(t *testing.T) {
127127
t.Run(testCase.Name, func(t *testing.T) {
128128
manager, err := Create(ifaceMock)
129129
t.Cleanup(func() {
130-
require.NoError(t, manager.Reset(nil))
130+
require.NoError(t, manager.Close(nil))
131131
})
132132
require.NoError(t, err)
133133
require.NoError(t, manager.Init(nil))

‎client/firewall/nftables/state_linux.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func (s *ShutdownState) Cleanup() error {
3939
return fmt.Errorf("create nftables manager: %w", err)
4040
}
4141

42-
if err := nft.Reset(nil); err != nil {
42+
if err := nft.Close(nil); err != nil {
4343
return fmt.Errorf("reset nftables manager: %w", err)
4444
}
4545

‎client/firewall/uspfilter/allow_netbird.go

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,11 @@ import (
88

99
log "github.com/sirupsen/logrus"
1010

11-
"github.com/netbirdio/netbird/client/firewall/uspfilter/conntrack"
1211
"github.com/netbirdio/netbird/client/internal/statemanager"
1312
)
1413

1514
// Reset firewall to the default state
16-
func (m *Manager) Reset(stateManager *statemanager.Manager) error {
15+
func (m *Manager) Close(stateManager *statemanager.Manager) error {
1716
m.mutex.Lock()
1817
defer m.mutex.Unlock()
1918

@@ -22,17 +21,14 @@ func (m *Manager) Reset(stateManager *statemanager.Manager) error {
2221

2322
if m.udpTracker != nil {
2423
m.udpTracker.Close()
25-
m.udpTracker = conntrack.NewUDPTracker(conntrack.DefaultUDPTimeout, m.logger)
2624
}
2725

2826
if m.icmpTracker != nil {
2927
m.icmpTracker.Close()
30-
m.icmpTracker = conntrack.NewICMPTracker(conntrack.DefaultICMPTimeout, m.logger)
3128
}
3229

3330
if m.tcpTracker != nil {
3431
m.tcpTracker.Close()
35-
m.tcpTracker = conntrack.NewTCPTracker(conntrack.DefaultTCPTimeout, m.logger)
3632
}
3733

3834
if m.forwarder != nil {
@@ -48,7 +44,7 @@ func (m *Manager) Reset(stateManager *statemanager.Manager) error {
4844
}
4945

5046
if m.nativeFirewall != nil {
51-
return m.nativeFirewall.Reset(stateManager)
47+
return m.nativeFirewall.Close(stateManager)
5248
}
5349
return nil
5450
}

‎client/firewall/uspfilter/allow_netbird_windows.go

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99

1010
log "github.com/sirupsen/logrus"
1111

12-
"github.com/netbirdio/netbird/client/firewall/uspfilter/conntrack"
1312
"github.com/netbirdio/netbird/client/internal/statemanager"
1413
)
1514

@@ -21,8 +20,8 @@ const (
2120
firewallRuleName = "Netbird"
2221
)
2322

24-
// Reset firewall to the default state
25-
func (m *Manager) Reset(*statemanager.Manager) error {
23+
// Close closes the firewall manager
24+
func (m *Manager) Close(*statemanager.Manager) error {
2625
m.mutex.Lock()
2726
defer m.mutex.Unlock()
2827

@@ -31,17 +30,14 @@ func (m *Manager) Reset(*statemanager.Manager) error {
3130

3231
if m.udpTracker != nil {
3332
m.udpTracker.Close()
34-
m.udpTracker = conntrack.NewUDPTracker(conntrack.DefaultUDPTimeout, m.logger)
3533
}
3634

3735
if m.icmpTracker != nil {
3836
m.icmpTracker.Close()
39-
m.icmpTracker = conntrack.NewICMPTracker(conntrack.DefaultICMPTimeout, m.logger)
4037
}
4138

4239
if m.tcpTracker != nil {
4340
m.tcpTracker.Close()
44-
m.tcpTracker = conntrack.NewTCPTracker(conntrack.DefaultTCPTimeout, m.logger)
4541
}
4642

4743
if m.forwarder != nil {

‎client/firewall/uspfilter/uspfilter_bench_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ func BenchmarkCoreFiltering(b *testing.B) {
160160
SetFilterFunc: func(device.PacketFilter) error { return nil },
161161
}, false)
162162
defer b.Cleanup(func() {
163-
require.NoError(b, manager.Reset(nil))
163+
require.NoError(b, manager.Close(nil))
164164
})
165165

166166
manager.wgNetwork = &net.IPNet{
@@ -205,7 +205,7 @@ func BenchmarkStateScaling(b *testing.B) {
205205
SetFilterFunc: func(device.PacketFilter) error { return nil },
206206
}, false)
207207
b.Cleanup(func() {
208-
require.NoError(b, manager.Reset(nil))
208+
require.NoError(b, manager.Close(nil))
209209
})
210210

211211
manager.wgNetwork = &net.IPNet{
@@ -253,7 +253,7 @@ func BenchmarkEstablishmentOverhead(b *testing.B) {
253253
SetFilterFunc: func(device.PacketFilter) error { return nil },
254254
}, false)
255255
b.Cleanup(func() {
256-
require.NoError(b, manager.Reset(nil))
256+
require.NoError(b, manager.Close(nil))
257257
})
258258

259259
manager.wgNetwork = &net.IPNet{
@@ -452,7 +452,7 @@ func BenchmarkRoutedNetworkReturn(b *testing.B) {
452452
SetFilterFunc: func(device.PacketFilter) error { return nil },
453453
}, false)
454454
b.Cleanup(func() {
455-
require.NoError(b, manager.Reset(nil))
455+
require.NoError(b, manager.Close(nil))
456456
})
457457

458458
// Setup scenario
@@ -579,7 +579,7 @@ func BenchmarkLongLivedConnections(b *testing.B) {
579579
SetFilterFunc: func(device.PacketFilter) error { return nil },
580580
}, false)
581581
defer b.Cleanup(func() {
582-
require.NoError(b, manager.Reset(nil))
582+
require.NoError(b, manager.Close(nil))
583583
})
584584

585585
manager.SetNetwork(&net.IPNet{
@@ -670,7 +670,7 @@ func BenchmarkShortLivedConnections(b *testing.B) {
670670
SetFilterFunc: func(device.PacketFilter) error { return nil },
671671
}, false)
672672
defer b.Cleanup(func() {
673-
require.NoError(b, manager.Reset(nil))
673+
require.NoError(b, manager.Close(nil))
674674
})
675675

676676
manager.SetNetwork(&net.IPNet{
@@ -789,7 +789,7 @@ func BenchmarkParallelLongLivedConnections(b *testing.B) {
789789
SetFilterFunc: func(device.PacketFilter) error { return nil },
790790
}, false)
791791
defer b.Cleanup(func() {
792-
require.NoError(b, manager.Reset(nil))
792+
require.NoError(b, manager.Close(nil))
793793
})
794794

795795
manager.SetNetwork(&net.IPNet{
@@ -877,7 +877,7 @@ func BenchmarkParallelShortLivedConnections(b *testing.B) {
877877
SetFilterFunc: func(device.PacketFilter) error { return nil },
878878
}, false)
879879
defer b.Cleanup(func() {
880-
require.NoError(b, manager.Reset(nil))
880+
require.NoError(b, manager.Close(nil))
881881
})
882882

883883
manager.SetNetwork(&net.IPNet{

‎client/firewall/uspfilter/uspfilter_filter_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func TestPeerACLFiltering(t *testing.T) {
3939
require.NotNil(t, manager)
4040

4141
t.Cleanup(func() {
42-
require.NoError(t, manager.Reset(nil))
42+
require.NoError(t, manager.Close(nil))
4343
})
4444

4545
manager.wgNetwork = wgNet
@@ -310,7 +310,7 @@ func setupRoutedManager(tb testing.TB, network string) *Manager {
310310
require.False(tb, manager.nativeRouter)
311311

312312
tb.Cleanup(func() {
313-
require.NoError(tb, manager.Reset(nil))
313+
require.NoError(tb, manager.Close(nil))
314314
})
315315

316316
return manager

‎client/firewall/uspfilter/uspfilter_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ func TestManagerReset(t *testing.T) {
254254
return
255255
}
256256

257-
err = m.Reset(nil)
257+
err = m.Close(nil)
258258
if err != nil {
259259
t.Errorf("failed to reset Manager: %v", err)
260260
return
@@ -333,7 +333,7 @@ func TestNotMatchByIP(t *testing.T) {
333333
return
334334
}
335335

336-
if err = m.Reset(nil); err != nil {
336+
if err = m.Close(nil); err != nil {
337337
t.Errorf("failed to reset Manager: %v", err)
338338
return
339339
}
@@ -352,7 +352,7 @@ func TestRemovePacketHook(t *testing.T) {
352352
t.Fatalf("Failed to create Manager: %s", err)
353353
}
354354
defer func() {
355-
require.NoError(t, manager.Reset(nil))
355+
require.NoError(t, manager.Close(nil))
356356
}()
357357

358358
// Add a UDP packet hook
@@ -403,7 +403,7 @@ func TestProcessOutgoingHooks(t *testing.T) {
403403
manager.udpTracker.Close()
404404
manager.udpTracker = conntrack.NewUDPTracker(100*time.Millisecond, logger)
405405
defer func() {
406-
require.NoError(t, manager.Reset(nil))
406+
require.NoError(t, manager.Close(nil))
407407
}()
408408

409409
manager.decoders = sync.Pool{
@@ -484,7 +484,7 @@ func TestUSPFilterCreatePerformance(t *testing.T) {
484484
time.Sleep(time.Second)
485485

486486
defer func() {
487-
if err := manager.Reset(nil); err != nil {
487+
if err := manager.Close(nil); err != nil {
488488
t.Errorf("clear the manager state: %v", err)
489489
}
490490
time.Sleep(time.Second)
@@ -530,7 +530,7 @@ func TestStatefulFirewall_UDPTracking(t *testing.T) {
530530
},
531531
}
532532
defer func() {
533-
require.NoError(t, manager.Reset(nil))
533+
require.NoError(t, manager.Close(nil))
534534
}()
535535

536536
// Set up packet parameters

0 commit comments

Comments
 (0)