diff --git a/client/firewall/iptables/manager_linux.go b/client/firewall/iptables/manager_linux.go index 696537dd8..19a313f7b 100644 --- a/client/firewall/iptables/manager_linux.go +++ b/client/firewall/iptables/manager_linux.go @@ -89,7 +89,7 @@ func (m *Manager) createIPv6Components(wgIface iFaceMapper, mtu uint16) error { } // Share the same IP forwarding state with the v4 router, since - // EnableIPForwarding controls both v4 and v6 sysctls. + // Forwarding refcounter is per-family but shared between v4 and v6 routers. m.router6.ipFwdState = m.router.ipFwdState m.aclMgr6, err = newAclManager(ip6Client, wgIface) @@ -402,15 +402,27 @@ func (m *Manager) SetLogLevel(log.Level) { } func (m *Manager) EnableRouting() error { - if err := m.router.ipFwdState.RequestForwarding(); err != nil { - return fmt.Errorf("enable IP forwarding: %w", err) + if err := m.router.ipFwdState.RequestForwarding(false); err != nil { + return fmt.Errorf("enable IPv4 forwarding: %w", err) + } + // Only flip v6 forwarding when the WG interface actually has v6, so that + // v4-only routing setups don't disable RA acceptance on the host. + if m.router6 != nil { + if err := m.router.ipFwdState.RequestForwarding(true); err != nil { + return fmt.Errorf("enable IPv6 forwarding: %w", err) + } } return nil } func (m *Manager) DisableRouting() error { - if err := m.router.ipFwdState.ReleaseForwarding(); err != nil { - return fmt.Errorf("disable IP forwarding: %w", err) + if err := m.router.ipFwdState.ReleaseForwarding(false); err != nil { + return fmt.Errorf("disable IPv4 forwarding: %w", err) + } + if m.router6 != nil { + if err := m.router.ipFwdState.ReleaseForwarding(true); err != nil { + return fmt.Errorf("disable IPv6 forwarding: %w", err) + } } return nil } diff --git a/client/firewall/iptables/router_linux.go b/client/firewall/iptables/router_linux.go index 290e5da1e..592d34f5c 100644 --- a/client/firewall/iptables/router_linux.go +++ b/client/firewall/iptables/router_linux.go @@ -101,7 +101,7 @@ func newRouter(iptablesClient *iptables.IPTables, wgIface iFaceMapper, mtu uint1 wgIface: wgIface, mtu: mtu, v6: iptablesClient.Proto() == iptables.ProtocolIPv6, - ipFwdState: ipfwdstate.NewIPForwardingState(), + ipFwdState: ipfwdstate.NewIPForwardingState(wgIface.Name()), } r.ipsetCounter = refcounter.New( @@ -763,7 +763,7 @@ func (r *router) updateState() { } func (r *router) AddDNATRule(rule firewall.ForwardRule) (firewall.Rule, error) { - if err := r.ipFwdState.RequestForwarding(); err != nil { + if err := r.ipFwdState.RequestForwarding(r.v6); err != nil { return nil, err } @@ -861,7 +861,7 @@ func (r *router) rollbackRules(rules map[string]ruleInfo) error { } func (r *router) DeleteDNATRule(rule firewall.Rule) error { - if err := r.ipFwdState.ReleaseForwarding(); err != nil { + if err := r.ipFwdState.ReleaseForwarding(r.v6); err != nil { log.Errorf("%v", err) } diff --git a/client/firewall/nftables/manager_linux.go b/client/firewall/nftables/manager_linux.go index fdc7c2f3c..109ab6ea4 100644 --- a/client/firewall/nftables/manager_linux.go +++ b/client/firewall/nftables/manager_linux.go @@ -105,8 +105,8 @@ func (m *Manager) createIPv6Components(tableName string, wgIface iFaceMapper, mt return fmt.Errorf("create v6 router: %w", err) } - // Share the same IP forwarding state with the v4 router, since - // EnableIPForwarding controls both v4 and v6 sysctls. + // Share the per-family forwarding refcounter with the v4 router so a v4 + // rule and a v6 rule against the same state machine cooperate cleanly. m.router6.ipFwdState = m.router.ipFwdState m.aclManager6, err = newAclManager(workTable6, wgIface, chainNameRoutingFw) @@ -530,15 +530,27 @@ func (m *Manager) SetLogLevel(log.Level) { } func (m *Manager) EnableRouting() error { - if err := m.router.ipFwdState.RequestForwarding(); err != nil { - return fmt.Errorf("enable IP forwarding: %w", err) + if err := m.router.ipFwdState.RequestForwarding(false); err != nil { + return fmt.Errorf("enable IPv4 forwarding: %w", err) + } + // Only flip v6 forwarding when the WG interface actually has v6, so that + // v4-only routing setups don't disable RA acceptance on the host. + if m.router6 != nil { + if err := m.router.ipFwdState.RequestForwarding(true); err != nil { + return fmt.Errorf("enable IPv6 forwarding: %w", err) + } } return nil } func (m *Manager) DisableRouting() error { - if err := m.router.ipFwdState.ReleaseForwarding(); err != nil { - return fmt.Errorf("disable IP forwarding: %w", err) + if err := m.router.ipFwdState.ReleaseForwarding(false); err != nil { + return fmt.Errorf("disable IPv4 forwarding: %w", err) + } + if m.router6 != nil { + if err := m.router.ipFwdState.ReleaseForwarding(true); err != nil { + return fmt.Errorf("disable IPv6 forwarding: %w", err) + } } return nil } diff --git a/client/firewall/nftables/router_linux.go b/client/firewall/nftables/router_linux.go index 4214455a9..701f6ddcd 100644 --- a/client/firewall/nftables/router_linux.go +++ b/client/firewall/nftables/router_linux.go @@ -93,7 +93,7 @@ func newRouter(workTable *nftables.Table, wgIface iFaceMapper, mtu uint16) (*rou rules: make(map[string]*nftables.Rule), af: familyForAddr(workTable.Family == nftables.TableFamilyIPv4), wgIface: wgIface, - ipFwdState: ipfwdstate.NewIPForwardingState(), + ipFwdState: ipfwdstate.NewIPForwardingState(wgIface.Name()), mtu: mtu, } @@ -1550,7 +1550,7 @@ func (r *router) refreshRulesMap() error { } func (r *router) AddDNATRule(rule firewall.ForwardRule) (firewall.Rule, error) { - if err := r.ipFwdState.RequestForwarding(); err != nil { + if err := r.ipFwdState.RequestForwarding(r.af.tableFamily == nftables.TableFamilyIPv6); err != nil { return nil, err } @@ -1778,7 +1778,7 @@ func (r *router) addDnatMasq(rule firewall.ForwardRule, protoNum uint8, ruleKey } func (r *router) DeleteDNATRule(rule firewall.Rule) error { - if err := r.ipFwdState.ReleaseForwarding(); err != nil { + if err := r.ipFwdState.ReleaseForwarding(r.af.tableFamily == nftables.TableFamilyIPv6); err != nil { log.Errorf("%v", err) } diff --git a/client/internal/debug/debug_linux.go b/client/internal/debug/debug_linux.go index 40d864eda..a36c0c0e7 100644 --- a/client/internal/debug/debug_linux.go +++ b/client/internal/debug/debug_linux.go @@ -844,6 +844,10 @@ func collectSysctls() string { []string{"net.ipv4.conf.all.src_valid_mark", "net.ipv4.conf.default.src_valid_mark"}, listInterfaceSysctls("ipv4", "src_valid_mark")..., )) + writeSysctlGroup(&builder, "accept_ra", append( + []string{"net.ipv6.conf.all.accept_ra", "net.ipv6.conf.default.accept_ra"}, + listInterfaceSysctls("ipv6", "accept_ra")..., + )) writeSysctlGroup(&builder, "conntrack", []string{ "net.netfilter.nf_conntrack_acct", "net.netfilter.nf_conntrack_tcp_loose", diff --git a/client/internal/routemanager/ipfwdstate/ipfwdstate.go b/client/internal/routemanager/ipfwdstate/ipfwdstate.go index 2be1c2ae7..6af8753e9 100644 --- a/client/internal/routemanager/ipfwdstate/ipfwdstate.go +++ b/client/internal/routemanager/ipfwdstate/ipfwdstate.go @@ -2,54 +2,106 @@ package ipfwdstate import ( "fmt" + "sync" log "github.com/sirupsen/logrus" "github.com/netbirdio/netbird/client/internal/routemanager/systemops" ) -// IPForwardingState is a struct that keeps track of the IP forwarding state. -// todo: read initial state of the IP forwarding from the system and reset the state based on it. -// todo: separate v4/v6 forwarding state, since the sysctls are independent -// (net.ipv4.ip_forward vs net.ipv6.conf.all.forwarding). Currently the nftables -// manager shares one instance between both routers, which works only because -// EnableIPForwarding enables both sysctls in a single call. +// IPForwardingState tracks per-family IP-forwarding sysctl enables with a +// refcount. v4 and v6 are independent so a v4-only routing setup doesn't flip +// net.ipv6.conf.all.forwarding, which on Linux disables RA acceptance by +// default and lets host RA-installed defaults silently expire. type IPForwardingState struct { - enabledCounter int + mu sync.Mutex + + v4Count int + v6Count int + + // wgIfaceName is excluded from the v6 accept_ra bump since the overlay + // interface doesn't carry upstream RAs. + wgIfaceName string + // v6Saved records the sysctl values captured when v6 forwarding was + // enabled (forwarding + per-interface accept_ra), restored on the last + // release. + v6Saved map[string]int } -func NewIPForwardingState() *IPForwardingState { - return &IPForwardingState{} +func NewIPForwardingState(wgIfaceName string) *IPForwardingState { + return &IPForwardingState{wgIfaceName: wgIfaceName} } -func (f *IPForwardingState) RequestForwarding() error { - if f.enabledCounter != 0 { - f.enabledCounter++ - return nil - } +// RequestForwarding bumps the per-family counter, enabling the underlying +// sysctl on the first request. +func (f *IPForwardingState) RequestForwarding(v6 bool) error { + f.mu.Lock() + defer f.mu.Unlock() - if err := systemops.EnableIPForwarding(); err != nil { - return fmt.Errorf("failed to enable IP forwarding with sysctl: %w", err) + if v6 { + return f.requestV6() } - f.enabledCounter = 1 - log.Info("IP forwarding enabled") + return f.requestV4() +} +// ReleaseForwarding decrements the per-family counter. The last v6 release +// also restores the sysctls v6 enable captured. v4 stays on: net.ipv4.ip_forward +// is a global knob other tools (docker, k8s, libvirt) co-own. +func (f *IPForwardingState) ReleaseForwarding(v6 bool) error { + f.mu.Lock() + defer f.mu.Unlock() + + if v6 { + return f.releaseV6() + } + f.releaseV4() return nil } -func (f *IPForwardingState) ReleaseForwarding() error { - if f.enabledCounter == 0 { - return nil +func (f *IPForwardingState) requestV4() error { + if f.v4Count == 0 { + if err := systemops.EnableV4IPForwarding(); err != nil { + return fmt.Errorf("enable IPv4 forwarding: %w", err) + } + log.Info("IPv4 forwarding enabled") } - - if f.enabledCounter > 1 { - f.enabledCounter-- - return nil - } - - // if failed to disable IP forwarding we anyway decrement the counter - f.enabledCounter = 0 - - // todo call systemops.DisableIPForwarding() + f.v4Count++ + return nil +} + +func (f *IPForwardingState) releaseV4() { + if f.v4Count > 0 { + f.v4Count-- + } +} + +func (f *IPForwardingState) requestV6() error { + if f.v6Count == 0 { + saved, err := systemops.EnableV6IPForwarding(f.wgIfaceName) + f.v6Saved = saved + if err != nil { + return fmt.Errorf("enable IPv6 forwarding: %w", err) + } + log.Info("IPv6 forwarding enabled") + } + f.v6Count++ + return nil +} + +func (f *IPForwardingState) releaseV6() error { + if f.v6Count == 0 { + return nil + } + f.v6Count-- + if f.v6Count > 0 { + return nil + } + + saved := f.v6Saved + f.v6Saved = nil + if err := systemops.DisableV6IPForwarding(saved); err != nil { + return fmt.Errorf("disable IPv6 forwarding: %w", err) + } + log.Info("IPv6 forwarding disabled") return nil } diff --git a/client/internal/routemanager/systemops/systemops_android.go b/client/internal/routemanager/systemops/systemops_android.go index 7cb8dae93..3840ad444 100644 --- a/client/internal/routemanager/systemops/systemops_android.go +++ b/client/internal/routemanager/systemops/systemops_android.go @@ -32,8 +32,17 @@ func (r *SysOps) removeFromRouteTable(netip.Prefix, Nexthop) error { return nil } -func EnableIPForwarding() error { - log.Infof("Enable IP forwarding is not implemented on %s", runtime.GOOS) +func EnableV4IPForwarding() error { + log.Infof("Enable IPv4 forwarding is not implemented on %s", runtime.GOOS) + return nil +} + +func EnableV6IPForwarding(string) (map[string]int, error) { + log.Infof("Enable IPv6 forwarding is not implemented on %s", runtime.GOOS) + return nil, nil +} + +func DisableV6IPForwarding(map[string]int) error { return nil } diff --git a/client/internal/routemanager/systemops/systemops_ios.go b/client/internal/routemanager/systemops/systemops_ios.go index 99a363371..fc1f58da5 100644 --- a/client/internal/routemanager/systemops/systemops_ios.go +++ b/client/internal/routemanager/systemops/systemops_ios.go @@ -58,8 +58,17 @@ func (r *SysOps) removeFromRouteTable(netip.Prefix, Nexthop) error { return nil } -func EnableIPForwarding() error { - log.Infof("Enable IP forwarding is not implemented on %s", runtime.GOOS) +func EnableV4IPForwarding() error { + log.Infof("Enable IPv4 forwarding is not implemented on %s", runtime.GOOS) + return nil +} + +func EnableV6IPForwarding(string) (map[string]int, error) { + log.Infof("Enable IPv6 forwarding is not implemented on %s", runtime.GOOS) + return nil, nil +} + +func DisableV6IPForwarding(map[string]int) error { return nil } diff --git a/client/internal/routemanager/systemops/systemops_linux.go b/client/internal/routemanager/systemops/systemops_linux.go index 8c6b7d9a9..8689a35d1 100644 --- a/client/internal/routemanager/systemops/systemops_linux.go +++ b/client/internal/routemanager/systemops/systemops_linux.go @@ -9,6 +9,7 @@ import ( "net" "net/netip" "os" + "strings" "syscall" "github.com/hashicorp/go-multierror" @@ -55,6 +56,11 @@ const ( ipv4ForwardingPath = "net.ipv4.ip_forward" // ipv6ForwardingPath is the path to the file containing the IPv6 forwarding setting. ipv6ForwardingPath = "net.ipv6.conf.all.forwarding" + // acceptRAInterfacePath toggles per-interface IPv6 RA acceptance. + // 1 (kernel default) accepts RAs only when forwarding is off; 2 keeps + // RA processing enabled even when forwarding is on, so RA-installed host + // defaults survive our v6 forwarding flip. + acceptRAInterfacePath = "net.ipv6.conf.%s.accept_ra" ) var ErrTableIDExists = errors.New("ID exists with different name") @@ -763,16 +769,81 @@ func flushRoutes(tableID, family int) error { return nberrors.FormatErrorOrNil(result) } -func EnableIPForwarding() error { +func EnableV4IPForwarding() error { if _, err := sysctl.Set(ipv4ForwardingPath, 1, false); err != nil { return err } - if _, err := sysctl.Set(ipv6ForwardingPath, 1, false); err != nil { - log.Warnf("failed to enable IPv6 forwarding: %v", err) - } return nil } +// EnableV6IPForwarding bumps accept_ra=2 on every non-loopback v6 interface +// before flipping forwarding=1, so RA-installed host defaults survive the flip. +// wgIfaceName is excluded since the overlay interface doesn't carry upstream RAs. +// +// The returned map records prior sysctl values for entries we actually changed +// (forwarding + per-interface accept_ra); DisableV6IPForwarding restores from +// it. Entries we found already at the target value are omitted so another +// process's sysctls aren't reset by our cleanup. +func EnableV6IPForwarding(wgIfaceName string) (map[string]int, error) { + saved := map[string]int{} + bumpAcceptRA(saved, wgIfaceName) + + oldVal, err := sysctl.Set(ipv6ForwardingPath, 1, false) + if err != nil { + return saved, err + } + if oldVal != 1 { + saved[ipv6ForwardingPath] = oldVal + } + return saved, nil +} + +// DisableV6IPForwarding restores every sysctl value EnableV6IPForwarding +// captured. v4 is intentionally not disabled: net.ipv4.ip_forward is a global +// knob other tools (docker, k8s, libvirt) co-own. +func DisableV6IPForwarding(saved map[string]int) error { + var result *multierror.Error + for key, value := range saved { + if _, err := sysctl.Set(key, value, false); err != nil { + result = multierror.Append(result, fmt.Errorf("restore %s: %w", key, err)) + } + } + return nberrors.FormatErrorOrNil(result) +} + +func bumpAcceptRA(saved map[string]int, wgIfaceName string) { + interfaces, err := net.Interfaces() + if err != nil { + log.Warnf("list interfaces for accept_ra: %v", err) + return + } + for _, intf := range interfaces { + if intf.Name == "lo" || intf.Name == wgIfaceName { + continue + } + bumpAcceptRAForInterface(saved, intf.Name) + } +} + +func bumpAcceptRAForInterface(saved map[string]int, name string) { + key := fmt.Sprintf(acceptRAInterfacePath, name) + procPath := "/proc/sys/" + strings.ReplaceAll(key, ".", "/") + if _, err := os.Stat(procPath); err != nil { + // No IPv6 stack on this interface. + return + } + // onlyIfOne=true: only bump from the kernel default; preserves admin + // overrides of 0 (don't accept RAs) or 2 (already what we want). + oldVal, err := sysctl.Set(key, 2, true) + if err != nil { + log.Warnf("bump %s: %v", key, err) + return + } + if oldVal != 2 { + saved[key] = oldVal + } +} + // entryExists checks if the specified ID or name already exists in the rt_tables file // and verifies if existing names start with "netbird_". func entryExists(file *os.File, id int) (bool, error) { diff --git a/client/internal/routemanager/systemops/systemops_nonlinux.go b/client/internal/routemanager/systemops/systemops_nonlinux.go index 016a62ebd..2c3377775 100644 --- a/client/internal/routemanager/systemops/systemops_nonlinux.go +++ b/client/internal/routemanager/systemops/systemops_nonlinux.go @@ -43,8 +43,17 @@ func (r *SysOps) RemoveVPNRoute(prefix netip.Prefix, intf *net.Interface) error return r.genericRemoveVPNRoute(prefix, intf) } -func EnableIPForwarding() error { - log.Infof("Enable IP forwarding is not implemented on %s", runtime.GOOS) +func EnableV4IPForwarding() error { + log.Infof("Enable IPv4 forwarding is not implemented on %s", runtime.GOOS) + return nil +} + +func EnableV6IPForwarding(string) (map[string]int, error) { + log.Infof("Enable IPv6 forwarding is not implemented on %s", runtime.GOOS) + return nil, nil +} + +func DisableV6IPForwarding(map[string]int) error { return nil }