Browse Source

cmd/tailscale/cli: add a risk message about rp_filter

We already present a health warning about this, but it is easy to miss
on a server when blackholing traffic makes it unreachable.

In addition to a health warning, present a risk message when exit node
is enabled.

Example:

```
$ tailscale up --exit-node=lizard
The following issues on your machine will likely make usage of exit nodes impossible:
- interface "ens4" has strict reverse-path filtering enabled
- interface "tailscale0" has strict reverse-path filtering enabled
Please set rp_filter=2 instead of rp_filter=1; see https://github.com/tailscale/tailscale/issues/3310
To skip this warning, use --accept-risk=linux-strict-rp-filter
$
```

Updates #3310

Signed-off-by: Anton Tolchanov <[email protected]>
Anton Tolchanov 9 months ago
parent
commit
db34cdcfe7

+ 19 - 0
client/local/local.go

@@ -788,6 +788,25 @@ 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.

+ 1 - 1
cmd/k8s-operator/depaware.txt

@@ -796,7 +796,7 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/
         tailscale.com/envknob/featureknob                            from tailscale.com/client/web+
         tailscale.com/feature                                        from tailscale.com/ipn/ipnext+
         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/internal/noiseconn                             from tailscale.com/control/controlclient

+ 20 - 1
cmd/tailscale/cli/risks.go

@@ -4,15 +4,18 @@
 package cli
 
 import (
+	"context"
 	"errors"
 	"flag"
 	"fmt"
 	"os"
 	"os/signal"
+	"runtime"
 	"strings"
 	"syscall"
 	"time"
 
+	"tailscale.com/ipn"
 	"tailscale.com/util/testenv"
 )
 
@@ -20,11 +23,12 @@ var (
 	riskTypes           []string
 	riskLoseSSH         = registerRiskType("lose-ssh")
 	riskMacAppConnector = registerRiskType("mac-app-connector")
+	riskStrictRPFilter  = registerRiskType("linux-strict-rp-filter")
 	riskAll             = registerRiskType("all")
 )
 
 const riskMacAppConnectorMessage = `
-You are trying to configure an app connector on macOS, which is not officially supported due to system limitations. This may result in performance and reliability issues. 
+You are trying to configure an app connector on macOS, which is not officially supported due to system limitations. This may result in performance and reliability issues.
 
 Do not use a macOS app connector for any mission-critical purposes. For the best experience, Linux is the only recommended platform for app connectors.
 `
@@ -89,3 +93,18 @@ func presentRiskToUser(riskType, riskMessage, acceptedRisks string) error {
 	printf("\r%s\r", strings.Repeat(" ", msgLen))
 	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
+}

+ 3 - 0
cmd/tailscale/cli/set.go

@@ -183,6 +183,9 @@ func runSet(ctx context.Context, args []string) (retErr error) {
 	}
 
 	warnOnAdvertiseRouts(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)

+ 3 - 0
cmd/tailscale/cli/up.go

@@ -481,6 +481,9 @@ func runUp(ctx context.Context, cmd string, args []string, upArgs upArgsT) (retE
 	}
 
 	warnOnAdvertiseRouts(ctx, prefs)
+	if err := checkExitNodeRisk(ctx, prefs, upArgs.acceptedRisks); err != nil {
+		return err
+	}
 
 	curPrefs, err := localClient.GetPrefs(ctx)
 	if err != nil {

+ 1 - 1
cmd/tailscaled/depaware.txt

@@ -281,7 +281,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/feature/tpm                                    from tailscale.com/feature/condregister
         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/internal/noiseconn                             from tailscale.com/control/controlclient
         tailscale.com/ipn                                            from tailscale.com/client/local+

+ 1 - 0
health/healthmsg/healthmsg.go

@@ -12,4 +12,5 @@ const (
 	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"
 )

+ 1 - 2
ipn/ipnlocal/local.go

@@ -4112,9 +4112,8 @@ func updateExitNodeUsageWarning(p ipn.PrefsView, state *netmon.State, healthTrac
 	var msg string
 	if p.ExitNodeIP().IsValid() || p.ExitNodeID() != "" {
 		warn, _ := netutil.CheckReversePathFiltering(state)
-		const comment = "please set rp_filter=2 instead of rp_filter=1; see https://github.com/tailscale/tailscale/issues/3310"
 		if len(warn) > 0 {
-			msg = fmt.Sprintf("%s: %v, %s", healthmsg.WarnExitNodeUsage, warn, comment)
+			msg = fmt.Sprintf("%s: %v, %s", healthmsg.WarnExitNodeUsage, warn, healthmsg.DisableRPFilter)
 		}
 	}
 	if len(msg) > 0 {

+ 93 - 65
ipn/localapi/localapi.go

@@ -32,6 +32,7 @@ import (
 	"tailscale.com/clientupdate"
 	"tailscale.com/drive"
 	"tailscale.com/envknob"
+	"tailscale.com/health/healthmsg"
 	"tailscale.com/hostinfo"
 	"tailscale.com/ipn"
 	"tailscale.com/ipn/ipnauth"
@@ -82,71 +83,72 @@ var handler = map[string]LocalAPIHandler{
 
 	// The other /localapi/v0/NAME handlers are exact matches and contain only NAME
 	// without a trailing slash:
-	"alpha-set-device-attrs":      (*Handler).serveSetDeviceAttrs, // see tailscale/corp#24690
-	"bugreport":                   (*Handler).serveBugReport,
-	"check-ip-forwarding":         (*Handler).serveCheckIPForwarding,
-	"check-prefs":                 (*Handler).serveCheckPrefs,
-	"check-udp-gro-forwarding":    (*Handler).serveCheckUDPGROForwarding,
-	"component-debug-logging":     (*Handler).serveComponentDebugLogging,
-	"debug":                       (*Handler).serveDebug,
-	"debug-derp-region":           (*Handler).serveDebugDERPRegion,
-	"debug-dial-types":            (*Handler).serveDebugDialTypes,
-	"debug-log":                   (*Handler).serveDebugLog,
-	"debug-packet-filter-matches": (*Handler).serveDebugPacketFilterMatches,
-	"debug-packet-filter-rules":   (*Handler).serveDebugPacketFilterRules,
-	"debug-peer-endpoint-changes": (*Handler).serveDebugPeerEndpointChanges,
-	"debug-portmap":               (*Handler).serveDebugPortmap,
-	"derpmap":                     (*Handler).serveDERPMap,
-	"dev-set-state-store":         (*Handler).serveDevSetStateStore,
-	"dial":                        (*Handler).serveDial,
-	"disconnect-control":          (*Handler).disconnectControl,
-	"dns-osconfig":                (*Handler).serveDNSOSConfig,
-	"dns-query":                   (*Handler).serveDNSQuery,
-	"drive/fileserver-address":    (*Handler).serveDriveServerAddr,
-	"drive/shares":                (*Handler).serveShares,
-	"goroutines":                  (*Handler).serveGoroutines,
-	"handle-push-message":         (*Handler).serveHandlePushMessage,
-	"id-token":                    (*Handler).serveIDToken,
-	"login-interactive":           (*Handler).serveLoginInteractive,
-	"logout":                      (*Handler).serveLogout,
-	"logtap":                      (*Handler).serveLogTap,
-	"metrics":                     (*Handler).serveMetrics,
-	"ping":                        (*Handler).servePing,
-	"pprof":                       (*Handler).servePprof,
-	"prefs":                       (*Handler).servePrefs,
-	"query-feature":               (*Handler).serveQueryFeature,
-	"reload-config":               (*Handler).reloadConfig,
-	"reset-auth":                  (*Handler).serveResetAuth,
-	"serve-config":                (*Handler).serveServeConfig,
-	"set-dns":                     (*Handler).serveSetDNS,
-	"set-expiry-sooner":           (*Handler).serveSetExpirySooner,
-	"set-gui-visible":             (*Handler).serveSetGUIVisible,
-	"set-push-device-token":       (*Handler).serveSetPushDeviceToken,
-	"set-udp-gro-forwarding":      (*Handler).serveSetUDPGROForwarding,
-	"set-use-exit-node-enabled":   (*Handler).serveSetUseExitNodeEnabled,
-	"start":                       (*Handler).serveStart,
-	"status":                      (*Handler).serveStatus,
-	"suggest-exit-node":           (*Handler).serveSuggestExitNode,
-	"tka/affected-sigs":           (*Handler).serveTKAAffectedSigs,
-	"tka/cosign-recovery-aum":     (*Handler).serveTKACosignRecoveryAUM,
-	"tka/disable":                 (*Handler).serveTKADisable,
-	"tka/force-local-disable":     (*Handler).serveTKALocalDisable,
-	"tka/generate-recovery-aum":   (*Handler).serveTKAGenerateRecoveryAUM,
-	"tka/init":                    (*Handler).serveTKAInit,
-	"tka/log":                     (*Handler).serveTKALog,
-	"tka/modify":                  (*Handler).serveTKAModify,
-	"tka/sign":                    (*Handler).serveTKASign,
-	"tka/status":                  (*Handler).serveTKAStatus,
-	"tka/submit-recovery-aum":     (*Handler).serveTKASubmitRecoveryAUM,
-	"tka/verify-deeplink":         (*Handler).serveTKAVerifySigningDeeplink,
-	"tka/wrap-preauth-key":        (*Handler).serveTKAWrapPreauthKey,
-	"update/check":                (*Handler).serveUpdateCheck,
-	"update/install":              (*Handler).serveUpdateInstall,
-	"update/progress":             (*Handler).serveUpdateProgress,
-	"upload-client-metrics":       (*Handler).serveUploadClientMetrics,
-	"usermetrics":                 (*Handler).serveUserMetrics,
-	"watch-ipn-bus":               (*Handler).serveWatchIPNBus,
-	"whois":                       (*Handler).serveWhoIs,
+	"alpha-set-device-attrs":       (*Handler).serveSetDeviceAttrs, // see tailscale/corp#24690
+	"bugreport":                    (*Handler).serveBugReport,
+	"check-ip-forwarding":          (*Handler).serveCheckIPForwarding,
+	"check-prefs":                  (*Handler).serveCheckPrefs,
+	"check-reverse-path-filtering": (*Handler).serveCheckReversePathFiltering,
+	"check-udp-gro-forwarding":     (*Handler).serveCheckUDPGROForwarding,
+	"component-debug-logging":      (*Handler).serveComponentDebugLogging,
+	"debug":                        (*Handler).serveDebug,
+	"debug-derp-region":            (*Handler).serveDebugDERPRegion,
+	"debug-dial-types":             (*Handler).serveDebugDialTypes,
+	"debug-log":                    (*Handler).serveDebugLog,
+	"debug-packet-filter-matches":  (*Handler).serveDebugPacketFilterMatches,
+	"debug-packet-filter-rules":    (*Handler).serveDebugPacketFilterRules,
+	"debug-peer-endpoint-changes":  (*Handler).serveDebugPeerEndpointChanges,
+	"debug-portmap":                (*Handler).serveDebugPortmap,
+	"derpmap":                      (*Handler).serveDERPMap,
+	"dev-set-state-store":          (*Handler).serveDevSetStateStore,
+	"dial":                         (*Handler).serveDial,
+	"disconnect-control":           (*Handler).disconnectControl,
+	"dns-osconfig":                 (*Handler).serveDNSOSConfig,
+	"dns-query":                    (*Handler).serveDNSQuery,
+	"drive/fileserver-address":     (*Handler).serveDriveServerAddr,
+	"drive/shares":                 (*Handler).serveShares,
+	"goroutines":                   (*Handler).serveGoroutines,
+	"handle-push-message":          (*Handler).serveHandlePushMessage,
+	"id-token":                     (*Handler).serveIDToken,
+	"login-interactive":            (*Handler).serveLoginInteractive,
+	"logout":                       (*Handler).serveLogout,
+	"logtap":                       (*Handler).serveLogTap,
+	"metrics":                      (*Handler).serveMetrics,
+	"ping":                         (*Handler).servePing,
+	"pprof":                        (*Handler).servePprof,
+	"prefs":                        (*Handler).servePrefs,
+	"query-feature":                (*Handler).serveQueryFeature,
+	"reload-config":                (*Handler).reloadConfig,
+	"reset-auth":                   (*Handler).serveResetAuth,
+	"serve-config":                 (*Handler).serveServeConfig,
+	"set-dns":                      (*Handler).serveSetDNS,
+	"set-expiry-sooner":            (*Handler).serveSetExpirySooner,
+	"set-gui-visible":              (*Handler).serveSetGUIVisible,
+	"set-push-device-token":        (*Handler).serveSetPushDeviceToken,
+	"set-udp-gro-forwarding":       (*Handler).serveSetUDPGROForwarding,
+	"set-use-exit-node-enabled":    (*Handler).serveSetUseExitNodeEnabled,
+	"start":                        (*Handler).serveStart,
+	"status":                       (*Handler).serveStatus,
+	"suggest-exit-node":            (*Handler).serveSuggestExitNode,
+	"tka/affected-sigs":            (*Handler).serveTKAAffectedSigs,
+	"tka/cosign-recovery-aum":      (*Handler).serveTKACosignRecoveryAUM,
+	"tka/disable":                  (*Handler).serveTKADisable,
+	"tka/force-local-disable":      (*Handler).serveTKALocalDisable,
+	"tka/generate-recovery-aum":    (*Handler).serveTKAGenerateRecoveryAUM,
+	"tka/init":                     (*Handler).serveTKAInit,
+	"tka/log":                      (*Handler).serveTKALog,
+	"tka/modify":                   (*Handler).serveTKAModify,
+	"tka/sign":                     (*Handler).serveTKASign,
+	"tka/status":                   (*Handler).serveTKAStatus,
+	"tka/submit-recovery-aum":      (*Handler).serveTKASubmitRecoveryAUM,
+	"tka/verify-deeplink":          (*Handler).serveTKAVerifySigningDeeplink,
+	"tka/wrap-preauth-key":         (*Handler).serveTKAWrapPreauthKey,
+	"update/check":                 (*Handler).serveUpdateCheck,
+	"update/install":               (*Handler).serveUpdateInstall,
+	"update/progress":              (*Handler).serveUpdateProgress,
+	"upload-client-metrics":        (*Handler).serveUploadClientMetrics,
+	"usermetrics":                  (*Handler).serveUserMetrics,
+	"watch-ipn-bus":                (*Handler).serveWatchIPNBus,
+	"whois":                        (*Handler).serveWhoIs,
 }
 
 // Register registers a new LocalAPI handler for the given name.
@@ -1175,6 +1177,32 @@ func (h *Handler) serveCheckIPForwarding(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)

+ 1 - 1
tsnet/depaware.txt

@@ -237,7 +237,7 @@ tailscale.com/tsnet dependencies: (generated by github.com/tailscale/depaware)
         tailscale.com/envknob/featureknob                            from tailscale.com/client/web+
         tailscale.com/feature                                        from tailscale.com/ipn/ipnext+
         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/noiseconn                             from tailscale.com/control/controlclient
         tailscale.com/ipn                                            from tailscale.com/client/local+