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 <[email protected]>

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.

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+

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
-}

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)

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: ")
 }

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+

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

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+

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"
 )

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 {

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)

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 {

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)
-}

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+