From 2c9ffdd188bd53ce43c8389f42594b2a8be6c390 Mon Sep 17 00:00:00 2001 From: Mike O'Driscoll Date: Wed, 4 Mar 2026 14:09:19 -0500 Subject: [PATCH] cmd/tailscale,ipn,net/netutil: remove rp_filter strict mode warnings (#18863) PR #18860 adds firewall rules in the mangle table to save outbound packet marks to conntrack and restore them on reply packets before the routing decision. When reply packets have their marks restored, the kernel uses the correct routing table (based on the mark) and the packets pass the rp_filter check. This makes the risk check and reverse path filtering warnings unnecessary. Updates #3310 Fixes tailscale/corp#37846 Signed-off-by: Mike O'Driscoll --- client/local/local.go | 19 ------ cmd/k8s-operator/depaware.txt | 2 +- cmd/tailscale/cli/risks.go | 19 ------ cmd/tailscale/cli/set.go | 4 +- cmd/tailscale/cli/up.go | 4 -- cmd/tailscaled/depaware-min.txt | 2 +- cmd/tailscaled/depaware.txt | 2 +- cmd/tsidp/depaware.txt | 2 +- health/healthmsg/healthmsg.go | 2 - ipn/ipnlocal/local.go | 30 --------- ipn/localapi/localapi.go | 30 --------- net/netutil/ip_forward.go | 104 -------------------------------- net/netutil/netutil_test.go | 21 ------- tsnet/depaware.txt | 2 +- 14 files changed, 6 insertions(+), 237 deletions(-) diff --git a/client/local/local.go b/client/local/local.go index 5794734f2..a7b8b83b1 100644 --- a/client/local/local.go +++ b/client/local/local.go @@ -818,25 +818,6 @@ func (lc *Client) CheckUDPGROForwarding(ctx context.Context) error { return nil } -// CheckReversePathFiltering asks the local Tailscale daemon whether strict -// reverse path filtering is enabled, which would break exit node usage on Linux. -func (lc *Client) CheckReversePathFiltering(ctx context.Context) error { - body, err := lc.get200(ctx, "/localapi/v0/check-reverse-path-filtering") - if err != nil { - return err - } - var jres struct { - Warning string - } - if err := json.Unmarshal(body, &jres); err != nil { - return fmt.Errorf("invalid JSON from check-reverse-path-filtering: %w", err) - } - if jres.Warning != "" { - return errors.New(jres.Warning) - } - return nil -} - // SetUDPGROForwarding enables UDP GRO forwarding for the main interface of this // node. This can be done to improve performance of tailnet nodes acting as exit // nodes or subnet routers. diff --git a/cmd/k8s-operator/depaware.txt b/cmd/k8s-operator/depaware.txt index d801c0285..c0cf0fd7c 100644 --- a/cmd/k8s-operator/depaware.txt +++ b/cmd/k8s-operator/depaware.txt @@ -813,7 +813,7 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/ tailscale.com/feature/syspolicy from tailscale.com/logpolicy tailscale.com/feature/useproxy from tailscale.com/feature/condregister/useproxy tailscale.com/health from tailscale.com/control/controlclient+ - tailscale.com/health/healthmsg from tailscale.com/ipn/ipnlocal+ + tailscale.com/health/healthmsg from tailscale.com/ipn/ipnlocal tailscale.com/hostinfo from tailscale.com/client/web+ tailscale.com/internal/client/tailscale from tailscale.com/cmd/k8s-operator+ tailscale.com/ipn from tailscale.com/client/local+ diff --git a/cmd/tailscale/cli/risks.go b/cmd/tailscale/cli/risks.go index 058eff1f8..1bd128d56 100644 --- a/cmd/tailscale/cli/risks.go +++ b/cmd/tailscale/cli/risks.go @@ -4,13 +4,10 @@ package cli import ( - "context" "errors" "flag" - "runtime" "strings" - "tailscale.com/ipn" "tailscale.com/util/prompt" "tailscale.com/util/testenv" ) @@ -19,7 +16,6 @@ var ( riskTypes []string riskLoseSSH = registerRiskType("lose-ssh") riskMacAppConnector = registerRiskType("mac-app-connector") - riskStrictRPFilter = registerRiskType("linux-strict-rp-filter") riskAll = registerRiskType("all") ) @@ -72,18 +68,3 @@ func presentRiskToUser(riskType, riskMessage, acceptedRisks string) error { return errAborted } - -// checkExitNodeRisk checks if the user is using an exit node on Linux and -// whether reverse path filtering is enabled. If so, it presents a risk message. -func checkExitNodeRisk(ctx context.Context, prefs *ipn.Prefs, acceptedRisks string) error { - if runtime.GOOS != "linux" { - return nil - } - if !prefs.ExitNodeIP.IsValid() && prefs.ExitNodeID == "" { - return nil - } - if err := localClient.CheckReversePathFiltering(ctx); err != nil { - return presentRiskToUser(riskStrictRPFilter, err.Error(), acceptedRisks) - } - return nil -} diff --git a/cmd/tailscale/cli/set.go b/cmd/tailscale/cli/set.go index 615900833..22d78641f 100644 --- a/cmd/tailscale/cli/set.go +++ b/cmd/tailscale/cli/set.go @@ -193,9 +193,7 @@ func runSet(ctx context.Context, args []string) (retErr error) { } warnOnAdvertiseRoutes(ctx, &maskedPrefs.Prefs) - if err := checkExitNodeRisk(ctx, &maskedPrefs.Prefs, setArgs.acceptedRisks); err != nil { - return err - } + var advertiseExitNodeSet, advertiseRoutesSet bool setFlagSet.Visit(func(f *flag.Flag) { updateMaskedPrefsFromUpOrSetFlag(maskedPrefs, f.Name) diff --git a/cmd/tailscale/cli/up.go b/cmd/tailscale/cli/up.go index d78cb2d44..79cc60ca2 100644 --- a/cmd/tailscale/cli/up.go +++ b/cmd/tailscale/cli/up.go @@ -543,9 +543,6 @@ func runUp(ctx context.Context, cmd string, args []string, upArgs upArgsT) (retE } warnOnAdvertiseRoutes(ctx, prefs) - if err := checkExitNodeRisk(ctx, prefs, upArgs.acceptedRisks); err != nil { - return err - } curPrefs, err := localClient.GetPrefs(ctx) if err != nil { @@ -834,7 +831,6 @@ func upWorthyWarning(s string) bool { return strings.Contains(s, healthmsg.TailscaleSSHOnBut) || strings.Contains(s, healthmsg.WarnAcceptRoutesOff) || strings.Contains(s, healthmsg.LockedOut) || - strings.Contains(s, healthmsg.WarnExitNodeUsage) || strings.Contains(s, healthmsg.InMemoryTailnetLockState) || strings.Contains(strings.ToLower(s), "update available: ") } diff --git a/cmd/tailscaled/depaware-min.txt b/cmd/tailscaled/depaware-min.txt index f97e0368c..fc39a980b 100644 --- a/cmd/tailscaled/depaware-min.txt +++ b/cmd/tailscaled/depaware-min.txt @@ -64,7 +64,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/feature/condregister/portmapper from tailscale.com/feature/condregister tailscale.com/feature/condregister/useproxy from tailscale.com/feature/condregister tailscale.com/health from tailscale.com/control/controlclient+ - tailscale.com/health/healthmsg from tailscale.com/ipn/ipnlocal+ + tailscale.com/health/healthmsg from tailscale.com/ipn/ipnlocal tailscale.com/hostinfo from tailscale.com/cmd/tailscaled+ tailscale.com/ipn from tailscale.com/cmd/tailscaled+ tailscale.com/ipn/conffile from tailscale.com/cmd/tailscaled+ diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 48a7d0949..efd1ea109 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -309,7 +309,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/feature/useproxy from tailscale.com/feature/condregister/useproxy tailscale.com/feature/wakeonlan from tailscale.com/feature/condregister tailscale.com/health from tailscale.com/control/controlclient+ - tailscale.com/health/healthmsg from tailscale.com/ipn/ipnlocal+ + tailscale.com/health/healthmsg from tailscale.com/ipn/ipnlocal tailscale.com/hostinfo from tailscale.com/client/web+ tailscale.com/ipn from tailscale.com/client/local+ W tailscale.com/ipn/auditlog from tailscale.com/cmd/tailscaled diff --git a/cmd/tsidp/depaware.txt b/cmd/tsidp/depaware.txt index 03f7e1f09..5016e568a 100644 --- a/cmd/tsidp/depaware.txt +++ b/cmd/tsidp/depaware.txt @@ -232,7 +232,7 @@ tailscale.com/cmd/tsidp dependencies: (generated by github.com/tailscale/depawar tailscale.com/feature/syspolicy from tailscale.com/logpolicy tailscale.com/feature/useproxy from tailscale.com/feature/condregister/useproxy tailscale.com/health from tailscale.com/control/controlclient+ - tailscale.com/health/healthmsg from tailscale.com/ipn/ipnlocal+ + tailscale.com/health/healthmsg from tailscale.com/ipn/ipnlocal tailscale.com/hostinfo from tailscale.com/client/web+ tailscale.com/internal/client/tailscale from tailscale.com/tsnet+ tailscale.com/ipn from tailscale.com/client/local+ diff --git a/health/healthmsg/healthmsg.go b/health/healthmsg/healthmsg.go index 3de885d53..c6efb0d57 100644 --- a/health/healthmsg/healthmsg.go +++ b/health/healthmsg/healthmsg.go @@ -11,7 +11,5 @@ const ( WarnAcceptRoutesOff = "Some peers are advertising routes but --accept-routes is false" TailscaleSSHOnBut = "Tailscale SSH enabled, but " // + ... something from caller LockedOut = "this node is locked out; it will not have connectivity until it is signed. For more info, see https://tailscale.com/s/locked-out" - WarnExitNodeUsage = "The following issues on your machine will likely make usage of exit nodes impossible" - DisableRPFilter = "Please set rp_filter=2 instead of rp_filter=1; see https://github.com/tailscale/tailscale/issues/3310" InMemoryTailnetLockState = "Tailnet Lock state is only being stored in-memory. Set --statedir to store state on disk, which is more secure. See https://tailscale.com/kb/1226/tailnet-lock#tailnet-lock-state" ) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index bae1e6639..9cb86642f 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -1031,7 +1031,6 @@ func (b *LocalBackend) linkChange(delta *netmon.ChangeDelta) { // If the local network configuration has changed, our filter may // need updating to tweak default routes. b.updateFilterLocked(prefs) - updateExitNodeUsageWarning(prefs, delta.CurrentState(), b.health) if buildfeatures.HasPeerAPIServer { cn := b.currentNode() @@ -4213,35 +4212,6 @@ func (b *LocalBackend) isDefaultServerLocked() bool { return prefs.ControlURLOrDefault(b.polc) == ipn.DefaultControlURL } -var exitNodeMisconfigurationWarnable = health.Register(&health.Warnable{ - Code: "exit-node-misconfiguration", - Title: "Exit node misconfiguration", - Severity: health.SeverityMedium, - Text: func(args health.Args) string { - return "Exit node misconfiguration: " + args[health.ArgError] - }, -}) - -// updateExitNodeUsageWarning updates a warnable meant to notify users of -// configuration issues that could break exit node usage. -func updateExitNodeUsageWarning(p ipn.PrefsView, state *netmon.State, healthTracker *health.Tracker) { - if !buildfeatures.HasUseExitNode { - return - } - var msg string - if p.ExitNodeIP().IsValid() || p.ExitNodeID() != "" { - warn, _ := netutil.CheckReversePathFiltering(state) - if len(warn) > 0 { - msg = fmt.Sprintf("%s: %v, %s", healthmsg.WarnExitNodeUsage, warn, healthmsg.DisableRPFilter) - } - } - if len(msg) > 0 { - healthTracker.SetUnhealthy(exitNodeMisconfigurationWarnable, health.Args{health.ArgError: msg}) - } else { - healthTracker.SetHealthy(exitNodeMisconfigurationWarnable) - } -} - func (b *LocalBackend) checkExitNodePrefsLocked(p *ipn.Prefs) error { tryingToUseExitNode := p.ExitNodeIP.IsValid() || p.ExitNodeID != "" if !tryingToUseExitNode { diff --git a/ipn/localapi/localapi.go b/ipn/localapi/localapi.go index dc558b36e..ed25e875d 100644 --- a/ipn/localapi/localapi.go +++ b/ipn/localapi/localapi.go @@ -28,7 +28,6 @@ import ( "tailscale.com/envknob" "tailscale.com/feature" "tailscale.com/feature/buildfeatures" - "tailscale.com/health/healthmsg" "tailscale.com/hostinfo" "tailscale.com/ipn" "tailscale.com/ipn/ipnauth" @@ -100,9 +99,6 @@ func init() { Register("check-udp-gro-forwarding", (*Handler).serveCheckUDPGROForwarding) Register("set-udp-gro-forwarding", (*Handler).serveSetUDPGROForwarding) } - if buildfeatures.HasUseExitNode && runtime.GOOS == "linux" { - Register("check-reverse-path-filtering", (*Handler).serveCheckReversePathFiltering) - } if buildfeatures.HasClientMetrics { Register("upload-client-metrics", (*Handler).serveUploadClientMetrics) } @@ -780,32 +776,6 @@ func (h *Handler) serveCheckSOMarkInUse(w http.ResponseWriter, r *http.Request) }) } -func (h *Handler) serveCheckReversePathFiltering(w http.ResponseWriter, r *http.Request) { - if !h.PermitRead { - http.Error(w, "reverse path filtering check access denied", http.StatusForbidden) - return - } - var warning string - - state := h.b.Sys().NetMon.Get().InterfaceState() - warn, err := netutil.CheckReversePathFiltering(state) - if err == nil && len(warn) > 0 { - var msg strings.Builder - msg.WriteString(healthmsg.WarnExitNodeUsage + ":\n") - for _, w := range warn { - msg.WriteString("- " + w + "\n") - } - msg.WriteString(healthmsg.DisableRPFilter) - warning = msg.String() - } - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(struct { - Warning string - }{ - Warning: warning, - }) -} - func (h *Handler) serveCheckUDPGROForwarding(w http.ResponseWriter, r *http.Request) { if !h.PermitRead { http.Error(w, "UDP GRO forwarding check access denied", http.StatusForbidden) diff --git a/net/netutil/ip_forward.go b/net/netutil/ip_forward.go index 0711953f5..bc0f1961d 100644 --- a/net/netutil/ip_forward.go +++ b/net/netutil/ip_forward.go @@ -5,7 +5,6 @@ package netutil import ( "bytes" - "errors" "fmt" "net/netip" "os" @@ -146,64 +145,6 @@ func CheckIPForwarding(routes []netip.Prefix, state *netmon.State) (warn, err er return nil, nil } -// CheckReversePathFiltering reports whether reverse path filtering is either -// disabled or set to 'loose' mode for exit node functionality on any -// interface. -// -// The routes should only be advertised routes, and should not contain the -// node's Tailscale IPs. -// -// This function returns an error if it is unable to determine whether reverse -// path filtering is enabled, or a warning describing configuration issues if -// reverse path fitering is non-functional or partly functional. -func CheckReversePathFiltering(state *netmon.State) (warn []string, err error) { - if runtime.GOOS != "linux" { - return nil, nil - } - - if state == nil { - return nil, errors.New("no link state") - } - - // The kernel uses the maximum value for rp_filter between the 'all' - // setting and each per-interface config, so we need to fetch both. - allSetting, err := reversePathFilterValueLinux("all") - if err != nil { - return nil, fmt.Errorf("reading global rp_filter value: %w", err) - } - - const ( - filtOff = 0 - filtStrict = 1 - filtLoose = 2 - ) - - // Because the kernel use the max rp_filter value, each interface will use 'loose', so we - // can abort early. - if allSetting == filtLoose { - return nil, nil - } - - for _, iface := range state.Interface { - if iface.IsLoopback() { - continue - } - - iSetting, err := reversePathFilterValueLinux(iface.Name) - if err != nil { - return nil, fmt.Errorf("reading interface rp_filter value for %q: %w", iface.Name, err) - } - // Perform the same max() that the kernel does - if allSetting > iSetting { - iSetting = allSetting - } - if iSetting == filtStrict { - warn = append(warn, fmt.Sprintf("interface %q has strict reverse-path filtering enabled", iface.Name)) - } - } - return warn, nil -} - // ipForwardSysctlKey returns the sysctl key for the given protocol and iface. // When the dotFormat parameter is true the output is formatted as `net.ipv4.ip_forward`, // else it is `net/ipv4/ip_forward` @@ -235,25 +176,6 @@ func ipForwardSysctlKey(format sysctlFormat, p protocol, iface string) string { return fmt.Sprintf(k, iface) } -// rpFilterSysctlKey returns the sysctl key for the given iface. -// -// Format controls whether the output is formatted as -// `net.ipv4.conf.iface.rp_filter` or `net/ipv4/conf/iface/rp_filter`. -func rpFilterSysctlKey(format sysctlFormat, iface string) string { - // No iface means all interfaces - if iface == "" { - iface = "all" - } - - k := "net/ipv4/conf/%s/rp_filter" - if format == dotFormat { - // Swap the delimiters. - iface = strings.ReplaceAll(iface, ".", "/") - k = strings.ReplaceAll(k, "/", ".") - } - return fmt.Sprintf(k, iface) -} - type sysctlFormat int const ( @@ -305,32 +227,6 @@ func ipForwardingEnabledLinux(p protocol, iface string) (bool, error) { return on, nil } -// reversePathFilterValueLinux reports the reverse path filter setting on Linux -// for the given interface. -// -// The iface param determines which interface to check against; the empty -// string means to check the global config. -// -// This function tries to look up the value directly from `/proc/sys`, and -// falls back to using the `sysctl` command on failure. -func reversePathFilterValueLinux(iface string) (int, error) { - k := rpFilterSysctlKey(slashFormat, iface) - bs, err := os.ReadFile(filepath.Join("/proc/sys", k)) - if err != nil { - // Fall back to the sysctl command - k := rpFilterSysctlKey(dotFormat, iface) - bs, err = exec.Command("sysctl", "-n", k).Output() - if err != nil { - return -1, fmt.Errorf("couldn't check %s (%v)", k, err) - } - } - v, err := strconv.Atoi(string(bytes.TrimSpace(bs))) - if err != nil { - return -1, fmt.Errorf("couldn't parse %s (%v)", k, err) - } - return v, nil -} - func ipForwardingEnabledSunOS(p protocol, iface string) (bool, error) { var proto string if p == ipv4 { diff --git a/net/netutil/netutil_test.go b/net/netutil/netutil_test.go index a512238d5..2c40d8d9e 100644 --- a/net/netutil/netutil_test.go +++ b/net/netutil/netutil_test.go @@ -8,9 +8,6 @@ import ( "net" "runtime" "testing" - - "tailscale.com/net/netmon" - "tailscale.com/util/eventbus" ) type conn struct { @@ -68,21 +65,3 @@ func TestIPForwardingEnabledLinux(t *testing.T) { t.Errorf("got true; want false") } } - -func TestCheckReversePathFiltering(t *testing.T) { - if runtime.GOOS != "linux" { - t.Skipf("skipping on %s", runtime.GOOS) - } - bus := eventbus.New() - defer bus.Close() - - netMon, err := netmon.New(bus, t.Logf) - if err != nil { - t.Fatal(err) - } - defer netMon.Close() - - warn, err := CheckReversePathFiltering(netMon.InterfaceState()) - t.Logf("err: %v", err) - t.Logf("warnings: %v", warn) -} diff --git a/tsnet/depaware.txt b/tsnet/depaware.txt index 8c81aa4d7..b61545d24 100644 --- a/tsnet/depaware.txt +++ b/tsnet/depaware.txt @@ -228,7 +228,7 @@ tailscale.com/tsnet dependencies: (generated by github.com/tailscale/depaware) tailscale.com/feature/syspolicy from tailscale.com/logpolicy tailscale.com/feature/useproxy from tailscale.com/feature/condregister/useproxy tailscale.com/health from tailscale.com/control/controlclient+ - tailscale.com/health/healthmsg from tailscale.com/ipn/ipnlocal+ + tailscale.com/health/healthmsg from tailscale.com/ipn/ipnlocal tailscale.com/hostinfo from tailscale.com/client/web+ tailscale.com/internal/client/tailscale from tailscale.com/tsnet+ tailscale.com/ipn from tailscale.com/client/local+