diff --git a/client/firewall/firewalld/firewalld.go b/client/firewall/firewalld/firewalld.go deleted file mode 100644 index 188ea61dd..000000000 --- a/client/firewall/firewalld/firewalld.go +++ /dev/null @@ -1,11 +0,0 @@ -// Package firewalld integrates with the firewalld daemon so NetBird can place -// its wg interface into firewalld's "trusted" zone. This is required because -// firewalld's nftables chains are created with NFT_CHAIN_OWNER on recent -// versions, which returns EPERM to any other process that tries to insert -// rules into them. The workaround mirrors what Tailscale does: let firewalld -// itself add the accept rules to its own chains by trusting the interface. -package firewalld - -// TrustedZone is the firewalld zone name used for interfaces whose traffic -// should bypass firewalld filtering. -const TrustedZone = "trusted" diff --git a/client/firewall/firewalld/firewalld_linux.go b/client/firewall/firewalld/firewalld_linux.go deleted file mode 100644 index 924a04b0a..000000000 --- a/client/firewall/firewalld/firewalld_linux.go +++ /dev/null @@ -1,260 +0,0 @@ -//go:build linux - -package firewalld - -import ( - "context" - "errors" - "fmt" - "os/exec" - "strings" - "sync" - "time" - - "github.com/godbus/dbus/v5" - log "github.com/sirupsen/logrus" -) - -const ( - dbusDest = "org.fedoraproject.FirewallD1" - dbusPath = "/org/fedoraproject/FirewallD1" - dbusRootIface = "org.fedoraproject.FirewallD1" - dbusZoneIface = "org.fedoraproject.FirewallD1.zone" - - errZoneAlreadySet = "ZONE_ALREADY_SET" - errAlreadyEnabled = "ALREADY_ENABLED" - errUnknownIface = "UNKNOWN_INTERFACE" - errNotEnabled = "NOT_ENABLED" - - // callTimeout bounds each individual DBus or firewall-cmd invocation. - // A fresh context is created for each call so a slow DBus probe can't - // exhaust the deadline before the firewall-cmd fallback gets to run. - callTimeout = 3 * time.Second -) - -var ( - errDBusUnavailable = errors.New("firewalld dbus unavailable") - - // trustLogOnce ensures the "added to trusted zone" message is logged at - // Info level only for the first successful add per process; repeat adds - // from other init paths are quieter. - trustLogOnce sync.Once - - parentCtxMu sync.RWMutex - parentCtx context.Context = context.Background() -) - -// SetParentContext installs a parent context whose cancellation aborts any -// in-flight TrustInterface call. It does not affect UntrustInterface, which -// always uses a fresh Background-rooted timeout so cleanup can still run -// during engine shutdown when the engine context is already cancelled. -func SetParentContext(ctx context.Context) { - parentCtxMu.Lock() - parentCtx = ctx - parentCtxMu.Unlock() -} - -func getParentContext() context.Context { - parentCtxMu.RLock() - defer parentCtxMu.RUnlock() - return parentCtx -} - -// TrustInterface places iface into firewalld's trusted zone if firewalld is -// running. It is idempotent and best-effort: errors are returned so callers -// can log, but a non-running firewalld is not an error. Only the first -// successful call per process logs at Info. Respects the parent context set -// via SetParentContext so startup-time cancellation unblocks it. -func TrustInterface(iface string) error { - parent := getParentContext() - if !isRunning(parent) { - return nil - } - if err := addTrusted(parent, iface); err != nil { - return fmt.Errorf("add %s to firewalld trusted zone: %w", iface, err) - } - trustLogOnce.Do(func() { - log.Infof("added %s to firewalld trusted zone", iface) - }) - log.Debugf("firewalld: ensured %s is in trusted zone", iface) - return nil -} - -// UntrustInterface removes iface from firewalld's trusted zone if firewalld -// is running. Idempotent. Uses a Background-rooted timeout so it still runs -// during shutdown after the engine context has been cancelled. -func UntrustInterface(iface string) error { - if !isRunning(context.Background()) { - return nil - } - if err := removeTrusted(context.Background(), iface); err != nil { - return fmt.Errorf("remove %s from firewalld trusted zone: %w", iface, err) - } - return nil -} - -func newCallContext(parent context.Context) (context.Context, context.CancelFunc) { - return context.WithTimeout(parent, callTimeout) -} - -func isRunning(parent context.Context) bool { - ctx, cancel := newCallContext(parent) - ok, err := isRunningDBus(ctx) - cancel() - if err == nil { - return ok - } - if errors.Is(err, errDBusUnavailable) || errors.Is(err, context.DeadlineExceeded) { - ctx, cancel = newCallContext(parent) - defer cancel() - return isRunningCLI(ctx) - } - return false -} - -func addTrusted(parent context.Context, iface string) error { - ctx, cancel := newCallContext(parent) - err := addDBus(ctx, iface) - cancel() - if err == nil { - return nil - } - if !errors.Is(err, errDBusUnavailable) { - log.Debugf("firewalld: dbus add failed, falling back to firewall-cmd: %v", err) - } - ctx, cancel = newCallContext(parent) - defer cancel() - return addCLI(ctx, iface) -} - -func removeTrusted(parent context.Context, iface string) error { - ctx, cancel := newCallContext(parent) - err := removeDBus(ctx, iface) - cancel() - if err == nil { - return nil - } - if !errors.Is(err, errDBusUnavailable) { - log.Debugf("firewalld: dbus remove failed, falling back to firewall-cmd: %v", err) - } - ctx, cancel = newCallContext(parent) - defer cancel() - return removeCLI(ctx, iface) -} - -func isRunningDBus(ctx context.Context) (bool, error) { - conn, err := dbus.SystemBus() - if err != nil { - return false, fmt.Errorf("%w: %v", errDBusUnavailable, err) - } - obj := conn.Object(dbusDest, dbusPath) - - var zone string - if err := obj.CallWithContext(ctx, dbusRootIface+".getDefaultZone", 0).Store(&zone); err != nil { - return false, fmt.Errorf("firewalld getDefaultZone: %w", err) - } - return true, nil -} - -func isRunningCLI(ctx context.Context) bool { - if _, err := exec.LookPath("firewall-cmd"); err != nil { - return false - } - return exec.CommandContext(ctx, "firewall-cmd", "--state").Run() == nil -} - -func addDBus(ctx context.Context, iface string) error { - conn, err := dbus.SystemBus() - if err != nil { - return fmt.Errorf("%w: %v", errDBusUnavailable, err) - } - obj := conn.Object(dbusDest, dbusPath) - - call := obj.CallWithContext(ctx, dbusZoneIface+".addInterface", 0, TrustedZone, iface) - if call.Err == nil { - return nil - } - - if dbusErrContains(call.Err, errAlreadyEnabled) { - return nil - } - - if dbusErrContains(call.Err, errZoneAlreadySet) { - move := obj.CallWithContext(ctx, dbusZoneIface+".changeZoneOfInterface", 0, TrustedZone, iface) - if move.Err != nil { - return fmt.Errorf("firewalld changeZoneOfInterface: %w", move.Err) - } - return nil - } - - return fmt.Errorf("firewalld addInterface: %w", call.Err) -} - -func removeDBus(ctx context.Context, iface string) error { - conn, err := dbus.SystemBus() - if err != nil { - return fmt.Errorf("%w: %v", errDBusUnavailable, err) - } - obj := conn.Object(dbusDest, dbusPath) - - call := obj.CallWithContext(ctx, dbusZoneIface+".removeInterface", 0, TrustedZone, iface) - if call.Err == nil { - return nil - } - - if dbusErrContains(call.Err, errUnknownIface) || dbusErrContains(call.Err, errNotEnabled) { - return nil - } - - return fmt.Errorf("firewalld removeInterface: %w", call.Err) -} - -func addCLI(ctx context.Context, iface string) error { - if _, err := exec.LookPath("firewall-cmd"); err != nil { - return fmt.Errorf("firewall-cmd not available: %w", err) - } - - // --change-interface (no --permanent) binds the interface for the - // current runtime only; we do not want membership to persist across - // reboots because netbird re-asserts it on every startup. - out, err := exec.CommandContext(ctx, - "firewall-cmd", "--zone="+TrustedZone, "--change-interface="+iface, - ).CombinedOutput() - if err != nil { - return fmt.Errorf("firewall-cmd change-interface: %w: %s", err, strings.TrimSpace(string(out))) - } - return nil -} - -func removeCLI(ctx context.Context, iface string) error { - if _, err := exec.LookPath("firewall-cmd"); err != nil { - return fmt.Errorf("firewall-cmd not available: %w", err) - } - - out, err := exec.CommandContext(ctx, - "firewall-cmd", "--zone="+TrustedZone, "--remove-interface="+iface, - ).CombinedOutput() - if err != nil { - msg := strings.TrimSpace(string(out)) - if strings.Contains(msg, errUnknownIface) || strings.Contains(msg, errNotEnabled) { - return nil - } - return fmt.Errorf("firewall-cmd remove-interface: %w: %s", err, msg) - } - return nil -} - -func dbusErrContains(err error, code string) bool { - if err == nil { - return false - } - var de dbus.Error - if errors.As(err, &de) { - for _, b := range de.Body { - if s, ok := b.(string); ok && strings.Contains(s, code) { - return true - } - } - } - return strings.Contains(err.Error(), code) -} diff --git a/client/firewall/firewalld/firewalld_linux_test.go b/client/firewall/firewalld/firewalld_linux_test.go deleted file mode 100644 index d812745fc..000000000 --- a/client/firewall/firewalld/firewalld_linux_test.go +++ /dev/null @@ -1,49 +0,0 @@ -//go:build linux - -package firewalld - -import ( - "errors" - "testing" - - "github.com/godbus/dbus/v5" -) - -func TestDBusErrContains(t *testing.T) { - tests := []struct { - name string - err error - code string - want bool - }{ - {"nil error", nil, errZoneAlreadySet, false}, - {"plain error match", errors.New("ZONE_ALREADY_SET: wt0"), errZoneAlreadySet, true}, - {"plain error miss", errors.New("something else"), errZoneAlreadySet, false}, - { - "dbus.Error body match", - dbus.Error{Name: "org.fedoraproject.FirewallD1.Exception", Body: []any{"ZONE_ALREADY_SET: wt0"}}, - errZoneAlreadySet, - true, - }, - { - "dbus.Error body miss", - dbus.Error{Name: "org.fedoraproject.FirewallD1.Exception", Body: []any{"INVALID_INTERFACE"}}, - errAlreadyEnabled, - false, - }, - { - "dbus.Error non-string body falls back to Error()", - dbus.Error{Name: "x", Body: []any{123}}, - "x", - true, - }, - } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - got := dbusErrContains(tc.err, tc.code) - if got != tc.want { - t.Fatalf("dbusErrContains(%v, %q) = %v; want %v", tc.err, tc.code, got, tc.want) - } - }) - } -} diff --git a/client/firewall/firewalld/firewalld_other.go b/client/firewall/firewalld/firewalld_other.go deleted file mode 100644 index cfa28221d..000000000 --- a/client/firewall/firewalld/firewalld_other.go +++ /dev/null @@ -1,25 +0,0 @@ -//go:build !linux - -package firewalld - -import "context" - -// SetParentContext is a no-op on non-Linux platforms because firewalld only -// runs on Linux. -func SetParentContext(context.Context) { - // intentionally empty: firewalld is a Linux-only daemon -} - -// TrustInterface is a no-op on non-Linux platforms because firewalld only -// runs on Linux. -func TrustInterface(string) error { - // intentionally empty: firewalld is a Linux-only daemon - return nil -} - -// UntrustInterface is a no-op on non-Linux platforms because firewalld only -// runs on Linux. -func UntrustInterface(string) error { - // intentionally empty: firewalld is a Linux-only daemon - return nil -} diff --git a/client/firewall/iptables/manager_linux.go b/client/firewall/iptables/manager_linux.go index 7d8cd7f8c..a1d4467d5 100644 --- a/client/firewall/iptables/manager_linux.go +++ b/client/firewall/iptables/manager_linux.go @@ -12,7 +12,6 @@ import ( log "github.com/sirupsen/logrus" 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" @@ -87,12 +86,6 @@ func (m *Manager) Init(stateManager *statemanager.Manager) error { log.Warnf("raw table not available, notrack rules will be disabled: %v", err) } - // Trust after all fatal init steps so a later failure doesn't leave the - // interface in firewalld's trusted zone without a corresponding Close. - if err := firewalld.TrustInterface(m.wgIface.Name()); err != nil { - log.Warnf("failed to trust interface in firewalld: %v", err) - } - // persist early to ensure cleanup of chains go func() { if err := stateManager.PersistState(context.Background()); err != nil { @@ -198,12 +191,6 @@ func (m *Manager) Close(stateManager *statemanager.Manager) error { merr = multierror.Append(merr, fmt.Errorf("reset router: %w", err)) } - // Appending to merr intentionally blocks DeleteState below so ShutdownState - // stays persisted and the crash-recovery path retries firewalld cleanup. - if err := firewalld.UntrustInterface(m.wgIface.Name()); err != nil { - merr = multierror.Append(merr, err) - } - // attempt to delete state only if all other operations succeeded if merr == nil { if err := stateManager.DeleteState(&ShutdownState{}); err != nil { @@ -230,11 +217,6 @@ func (m *Manager) AllowNetbird() error { if err != nil { return fmt.Errorf("allow netbird interface traffic: %w", err) } - - if err := firewalld.TrustInterface(m.wgIface.Name()); err != nil { - log.Warnf("failed to trust interface in firewalld: %v", err) - } - return nil } diff --git a/client/firewall/nftables/manager_linux.go b/client/firewall/nftables/manager_linux.go index 8cd5cc6b3..0b5b61e04 100644 --- a/client/firewall/nftables/manager_linux.go +++ b/client/firewall/nftables/manager_linux.go @@ -14,7 +14,6 @@ import ( log "github.com/sirupsen/logrus" "golang.org/x/sys/unix" - "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" @@ -218,10 +217,6 @@ func (m *Manager) AllowNetbird() error { 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 } diff --git a/client/firewall/nftables/router_linux.go b/client/firewall/nftables/router_linux.go index 8cc0d2792..904daf7cb 100644 --- a/client/firewall/nftables/router_linux.go +++ b/client/firewall/nftables/router_linux.go @@ -19,7 +19,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" nbid "github.com/netbirdio/netbird/client/internal/acl/id" "github.com/netbirdio/netbird/client/internal/routemanager/ipfwdstate" @@ -41,8 +40,6 @@ const ( chainNameForward = "FORWARD" chainNameMangleForward = "netbird-mangle-forward" - firewalldTableName = "firewalld" - userDataAcceptForwardRuleIif = "frwacceptiif" userDataAcceptForwardRuleOif = "frwacceptoif" userDataAcceptInputRule = "inputaccept" @@ -136,10 +133,6 @@ func (r *router) Reset() error { merr = multierror.Append(merr, fmt.Errorf("remove accept filter rules: %w", err)) } - if err := firewalld.UntrustInterface(r.wgIface.Name()); err != nil { - merr = multierror.Append(merr, err) - } - if err := r.removeNatPreroutingRules(); err != nil { merr = multierror.Append(merr, fmt.Errorf("remove filter prerouting rules: %w", err)) } @@ -287,10 +280,6 @@ func (r *router) createContainers() error { log.Errorf("failed to add accept rules for the forward chain: %s", err) } - if err := firewalld.TrustInterface(r.wgIface.Name()); err != nil { - log.Warnf("failed to trust interface in firewalld: %v", err) - } - if err := r.refreshRulesMap(); err != nil { log.Errorf("failed to refresh rules: %s", err) } @@ -1330,13 +1319,6 @@ func (r *router) isExternalChain(chain *nftables.Chain) bool { return false } - // Skip firewalld-owned chains. Firewalld creates its chains with the - // NFT_CHAIN_OWNER flag, so inserting rules into them returns EPERM. - // We delegate acceptance to firewalld by trusting the interface instead. - if chain.Table.Name == firewalldTableName { - return false - } - // Skip all iptables-managed tables in the ip family if chain.Table.Family == nftables.TableFamilyIPv4 && isIptablesTable(chain.Table.Name) { return false diff --git a/client/firewall/uspfilter/allow_netbird.go b/client/firewall/uspfilter/allow_netbird.go index b120cdf12..6a6533344 100644 --- a/client/firewall/uspfilter/allow_netbird.go +++ b/client/firewall/uspfilter/allow_netbird.go @@ -3,9 +3,6 @@ package uspfilter import ( - log "github.com/sirupsen/logrus" - - "github.com/netbirdio/netbird/client/firewall/firewalld" "github.com/netbirdio/netbird/client/internal/statemanager" ) @@ -19,9 +16,6 @@ func (m *Manager) Close(stateManager *statemanager.Manager) error { if m.nativeFirewall != nil { return m.nativeFirewall.Close(stateManager) } - if err := firewalld.UntrustInterface(m.wgIface.Name()); err != nil { - log.Warnf("failed to untrust interface in firewalld: %v", err) - } return nil } @@ -30,8 +24,5 @@ func (m *Manager) AllowNetbird() error { if m.nativeFirewall != nil { return m.nativeFirewall.AllowNetbird() } - if err := firewalld.TrustInterface(m.wgIface.Name()); err != nil { - log.Warnf("failed to trust interface in firewalld: %v", err) - } return nil } diff --git a/client/firewall/uspfilter/common/iface.go b/client/firewall/uspfilter/common/iface.go index 9c06eb3f7..7296953db 100644 --- a/client/firewall/uspfilter/common/iface.go +++ b/client/firewall/uspfilter/common/iface.go @@ -9,7 +9,6 @@ import ( // IFaceMapper defines subset methods of interface required for manager type IFaceMapper interface { - Name() string SetFilter(device.PacketFilter) error Address() wgaddr.Address GetWGDevice() *wgdevice.Device diff --git a/client/firewall/uspfilter/filter_test.go b/client/firewall/uspfilter/filter_test.go index 5fb9fef0e..39e8efa2c 100644 --- a/client/firewall/uspfilter/filter_test.go +++ b/client/firewall/uspfilter/filter_test.go @@ -31,20 +31,12 @@ var logger = log.NewFromLogrus(logrus.StandardLogger()) var flowLogger = netflow.NewManager(nil, []byte{}, nil).GetLogger() type IFaceMock struct { - NameFunc func() string SetFilterFunc func(device.PacketFilter) error AddressFunc func() wgaddr.Address GetWGDeviceFunc func() *wgdevice.Device GetDeviceFunc func() *device.FilteredDevice } -func (i *IFaceMock) Name() string { - if i.NameFunc == nil { - return "wgtest" - } - return i.NameFunc() -} - func (i *IFaceMock) GetWGDevice() *wgdevice.Device { if i.GetWGDeviceFunc == nil { return nil diff --git a/client/internal/engine.go b/client/internal/engine.go index 8d7e02bd5..09d80a87d 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -26,7 +26,6 @@ import ( nberrors "github.com/netbirdio/netbird/client/errors" "github.com/netbirdio/netbird/client/firewall" - "github.com/netbirdio/netbird/client/firewall/firewalld" firewallManager "github.com/netbirdio/netbird/client/firewall/manager" "github.com/netbirdio/netbird/client/iface" "github.com/netbirdio/netbird/client/iface/device" @@ -605,8 +604,6 @@ func (e *Engine) createFirewall() error { return nil } - firewalld.SetParentContext(e.ctx) - var err error e.firewall, err = firewall.NewFirewall(e.wgInterface, e.stateManager, e.flowManager.GetLogger(), e.config.DisableServerRoutes, e.config.MTU) if err != nil {