[client] Cleanup firewall state on startup

client/firewall/create.go

@@ -11,10 +11,11 @@ import (
 
 	firewall "github.com/netbirdio/netbird/client/firewall/manager"
 	"github.com/netbirdio/netbird/client/firewall/uspfilter"
+	"github.com/netbirdio/netbird/client/internal/statemanager"
 )
 
 // NewFirewall creates a firewall manager instance
-func NewFirewall(context context.Context, iface IFaceMapper) (firewall.Manager, error) {
+func NewFirewall(_ context.Context, iface IFaceMapper, _ *statemanager.Manager) (firewall.Manager, error) {
 	if !iface.IsUserspaceBind() {
 		return nil, fmt.Errorf("not implemented for this OS: %s", runtime.GOOS)
 	}

client/firewall/create_linux.go

@@ -15,6 +15,7 @@ import (
 	firewall "github.com/netbirdio/netbird/client/firewall/manager"
 	nbnftables "github.com/netbirdio/netbird/client/firewall/nftables"
 	"github.com/netbirdio/netbird/client/firewall/uspfilter"
+	"github.com/netbirdio/netbird/client/internal/statemanager"
 )
 
 const (
@@ -32,7 +33,7 @@ const SKIP_NFTABLES_ENV = "NB_SKIP_NFTABLES_CHECK"
 // FWType is the type for the firewall type
 type FWType int
 
-func NewFirewall(context context.Context, iface IFaceMapper) (firewall.Manager, error) {
+func NewFirewall(context context.Context, iface IFaceMapper, stateManager *statemanager.Manager) (firewall.Manager, error) {
 	// on the linux system we try to user nftables or iptables
 	// in any case, because we need to allow netbird interface traffic
 	// so we use AllowNetbird traffic from these firewall managers
@@ -58,6 +59,12 @@ func NewFirewall(context context.Context, iface IFaceMapper) (firewall.Manager,
 		log.Info("no firewall manager found, trying to use userspace packet filtering firewall")
 	}
 
+	if fm != nil {
+		if err := fm.Init(stateManager); err != nil {
+			log.Errorf("failed to init nftables manager: %s", err)
+		}
+	}
+
 	if iface.IsUserspaceBind() {
 		var errUsp error
 		if errFw == nil {

client/firewall/iptables/acl_linux.go

@@ -11,6 +11,7 @@ import (
 	log "github.com/sirupsen/logrus"
 
 	firewall "github.com/netbirdio/netbird/client/firewall/manager"
+	"github.com/netbirdio/netbird/client/internal/statemanager"
 	nbnet "github.com/netbirdio/netbird/util/net"
 )
 
@@ -22,6 +23,8 @@ const (
 	chainNameOutputRules = "NETBIRD-ACL-OUTPUT"
 )
 
+type aclEntries map[string][][]string
+
 type entry struct {
 	spec     []string
 	position int
@@ -32,9 +35,11 @@ type aclManager struct {
 	wgIface            iFaceMapper
 	routingFwChainName string
 
-	entries         map[string][][]string
+	entries         aclEntries
 	optionalEntries map[string][]entry
 	ipsetStore      *ipsetStore
+
+	stateManager *statemanager.Manager
 }
 
 func newAclManager(iptablesClient *iptables.IPTables, wgIface iFaceMapper, routingFwChainName string) (*aclManager, error) {
@@ -48,24 +53,30 @@ func newAclManager(iptablesClient *iptables.IPTables, wgIface iFaceMapper, routi
 		ipsetStore:      newIpsetStore(),
 	}
 
-	err := ipset.Init()
-	if err != nil {
-		return nil, fmt.Errorf("failed to init ipset: %w", err)
+	if err := ipset.Init(); err != nil {
+		return nil, fmt.Errorf("init ipset: %w", err)
 	}
 
+	return m, nil
+}
+
+func (m *aclManager) init(stateManager *statemanager.Manager) error {
+	m.stateManager = stateManager
+
 	m.seedInitialEntries()
 	m.seedInitialOptionalEntries()
 
-	err = m.cleanChains()
-	if err != nil {
-		return nil, err
+	if err := m.cleanChains(); err != nil {
+		return fmt.Errorf("clean chains: %w", err)
 	}
 
-	err = m.createDefaultChains()
-	if err != nil {
-		return nil, err
+	if err := m.createDefaultChains(); err != nil {
+		return fmt.Errorf("create default chains: %w", err)
 	}
-	return m, nil
+
+	m.updateState()
+
+	return nil
 }
 
 func (m *aclManager) AddPeerFiltering(
@@ -146,6 +157,8 @@ func (m *aclManager) AddPeerFiltering(
 		chain:     chain,
 	}
 
+	m.updateState()
+
 	return []firewall.Rule{rule}, nil
 }
 
@@ -180,15 +193,23 @@ func (m *aclManager) DeletePeerRule(rule firewall.Rule) error {
 		}
 	}
 
-	err := m.iptablesClient.Delete(tableName, r.chain, r.specs...)
-	if err != nil {
-		log.Debugf("failed to delete rule, %s, %v: %s", r.chain, r.specs, err)
+	if err := m.iptablesClient.Delete(tableName, r.chain, r.specs...); err != nil {
+		return fmt.Errorf("failed to delete rule: %s, %v: %w", r.chain, r.specs, err)
 	}
-	return err
+
+	m.updateState()
+
+	return nil
 }
 
 func (m *aclManager) Reset() error {
-	return m.cleanChains()
+	if err := m.cleanChains(); err != nil {
+		return fmt.Errorf("clean chains: %w", err)
+	}
+
+	m.updateState()
+
+	return nil
 }
 
 // todo write less destructive cleanup mechanism
@@ -348,6 +369,32 @@ func (m *aclManager) appendToEntries(chainName string, spec []string) {
 	m.entries[chainName] = append(m.entries[chainName], spec)
 }
 
+func (m *aclManager) updateState() {
+	if m.stateManager == nil {
+		return
+	}
+
+	var currentState *ShutdownState
+	if existing := m.stateManager.GetState(&ShutdownState{}); existing != nil {
+		if existingState, ok := existing.(*ShutdownState); ok {
+			currentState = existingState
+		}
+	}
+	if currentState == nil {
+		currentState = &ShutdownState{}
+	}
+
+	currentState.Lock()
+	defer currentState.Unlock()
+
+	currentState.ACLEntries = m.entries
+	currentState.ACLIPsetStore = m.ipsetStore
+
+	if err := m.stateManager.UpdateState(currentState); err != nil {
+		log.Errorf("failed to update state: %v", err)
+	}
+}
+
 // filterRuleSpecs returns the specs of a filtering rule
 func filterRuleSpecs(
 	ip net.IP, protocol string, sPort, dPort string, direction firewall.RuleDirection, action firewall.Action, ipsetName string,

client/firewall/iptables/manager_linux.go

@@ -8,10 +8,13 @@ import (
 	"sync"
 
 	"github.com/coreos/go-iptables/iptables"
+	"github.com/hashicorp/go-multierror"
 	log "github.com/sirupsen/logrus"
 
+	nberrors "github.com/netbirdio/netbird/client/errors"
 	firewall "github.com/netbirdio/netbird/client/firewall/manager"
 	"github.com/netbirdio/netbird/client/iface"
+	"github.com/netbirdio/netbird/client/internal/statemanager"
 )
 
 // Manager of iptables firewall
@@ -36,7 +39,7 @@ type iFaceMapper interface {
 func Create(context context.Context, wgIface iFaceMapper) (*Manager, error) {
 	iptablesClient, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
 	if err != nil {
-		return nil, fmt.Errorf("iptables is not installed in the system or not supported")
+		return nil, fmt.Errorf("init iptables: %w", err)
 	}
 
 	m := &Manager{
@@ -46,18 +49,47 @@ func Create(context context.Context, wgIface iFaceMapper) (*Manager, error) {
 
 	m.router, err = newRouter(context, iptablesClient, wgIface)
 	if err != nil {
-		log.Debugf("failed to initialize route related chains: %s", err)
-		return nil, err
+		return nil, fmt.Errorf("create router: %w", err)
 	}
+
 	m.aclMgr, err = newAclManager(iptablesClient, wgIface, chainRTFWD)
 	if err != nil {
-		log.Debugf("failed to initialize ACL manager: %s", err)
-		return nil, err
+		return nil, fmt.Errorf("create acl manager: %w", err)
 	}
 
 	return m, nil
 }
 
+func (m *Manager) Init(stateManager *statemanager.Manager) error {
+	state := &ShutdownState{
+		InterfaceState: &InterfaceState{
+			NameStr:       m.wgIface.Name(),
+			WGAddress:     m.wgIface.Address(),
+			UserspaceBind: m.wgIface.IsUserspaceBind(),
+		},
+	}
+	stateManager.RegisterState(state)
+	if err := stateManager.UpdateState(state); err != nil {
+		log.Errorf("failed to update state: %v", err)
+	}
+
+	if err := m.router.init(stateManager); err != nil {
+		return fmt.Errorf("router init: %w", err)
+	}
+
+	if err := m.aclMgr.init(stateManager); err != nil {
+		// TODO: cleanup router
+		return fmt.Errorf("acl manager init: %w", err)
+	}
+
+	// persist early to ensure cleanup of chains
+	if err := stateManager.PersistState(context.Background()); err != nil {
+		log.Errorf("failed to persist state: %v", err)
+	}
+
+	return nil
+}
+
 // AddPeerFiltering adds a rule to the firewall
 //
 // Comment will be ignored because some system this feature is not supported
@@ -133,20 +165,27 @@ func (m *Manager) SetLegacyManagement(isLegacy bool) error {
 }
 
 // Reset firewall to the default state
-func (m *Manager) Reset() error {
+func (m *Manager) Reset(stateManager *statemanager.Manager) error {
 	m.mutex.Lock()
 	defer m.mutex.Unlock()
 
-	errAcl := m.aclMgr.Reset()
-	if errAcl != nil {
-		log.Errorf("failed to clean up ACL rules from firewall: %s", errAcl)
+	var merr *multierror.Error
+
+	if err := m.aclMgr.Reset(); err != nil {
+		merr = multierror.Append(merr, fmt.Errorf("reset acl manager: %w", err))
+	}
+	if err := m.router.Reset(); err != nil {
+		merr = multierror.Append(merr, fmt.Errorf("reset router: %w", err))
 	}
-	errMgr := m.router.Reset()
-	if errMgr != nil {
-		log.Errorf("failed to clean up router rules from firewall: %s", errMgr)
-		return errMgr
+
+	// attempt to delete state only if all other operations succeeded
+	if merr == nil {
+		if err := stateManager.DeleteState(&ShutdownState{}); err != nil {
+			merr = multierror.Append(merr, fmt.Errorf("delete state: %w", err))
+		}
 	}
-	return errAcl
+
+	return nberrors.FormatErrorOrNil(merr)
 }
 
 // AllowNetbird allows netbird interface traffic

client/firewall/iptables/manager_linux_test.go

@@ -58,11 +58,12 @@ func TestIptablesManager(t *testing.T) {
 	// just check on the local interface
 	manager, err := Create(context.Background(), ifaceMock)
 	require.NoError(t, err)
+	require.NoError(t, manager.Init(nil))
 
 	time.Sleep(time.Second)
 
 	defer func() {
-		err := manager.Reset()
+		err := manager.Reset(nil)
 		require.NoError(t, err, "clear the manager state")
 
 		time.Sleep(time.Second)
@@ -122,7 +123,7 @@ func TestIptablesManager(t *testing.T) {
 		_, err = manager.AddPeerFiltering(ip, "udp", nil, port, fw.RuleDirectionOUT, fw.ActionAccept, "", "accept Fake DNS traffic")
 		require.NoError(t, err, "failed to add rule")
 
-		err = manager.Reset()
+		err = manager.Reset(nil)
 		require.NoError(t, err, "failed to reset")
 
 		ok, err := ipv4Client.ChainExists("filter", chainNameInputRules)
@@ -156,11 +157,12 @@ func TestIptablesManagerIPSet(t *testing.T) {
 	// just check on the local interface
 	manager, err := Create(context.Background(), mock)
 	require.NoError(t, err)
+	require.NoError(t, manager.Init(nil))
 
 	time.Sleep(time.Second)
 
 	defer func() {
-		err := manager.Reset()
+		err := manager.Reset(nil)
 		require.NoError(t, err, "clear the manager state")
 
 		time.Sleep(time.Second)
@@ -219,7 +221,7 @@ func TestIptablesManagerIPSet(t *testing.T) {
 	})
 
 	t.Run("reset check", func(t *testing.T) {
-		err = manager.Reset()
+		err = manager.Reset(nil)
 		require.NoError(t, err, "failed to reset")
 	})
 }
@@ -253,10 +255,11 @@ func TestIptablesCreatePerformance(t *testing.T) {
 			// just check on the local interface
 			manager, err := Create(context.Background(), mock)
 			require.NoError(t, err)
+			require.NoError(t, manager.Init(nil))
 			time.Sleep(time.Second)
 
 			defer func() {
-				err := manager.Reset()
+				err := manager.Reset(nil)
 				require.NoError(t, err, "clear the manager state")
 
 				time.Sleep(time.Second)