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+