ipn/ipnlocal: show warnings about reverse path filtering

Updates tailscale/tailscale#4432
Depends on tailscale/tailscale-www#1656

Change-Id: I519656b70d07a61b9308aad196fba982fc3ca8fc
Signed-off-by: Andrew Dunham <andrew@tailscale.com>

client/tailscale/localclient.go

@@ -427,6 +427,25 @@ func (lc *LocalClient) CheckIPForwarding(ctx context.Context) error {
 	return nil
 }
 
+// CheckReversePathFiltering asks the local Tailscale daemon whether it looks
+// like the machine has a bad configuration for reverse path filtering.
+func (lc *LocalClient) 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
+}
+
 // CheckPrefs validates the provided preferences, without making any changes.
 //
 // The CLI uses this before a Start call to fail fast if the preferences won't

cmd/tailscale/cli/up.go

@@ -469,6 +469,9 @@ func runUp(ctx context.Context, args []string) (retErr error) {
 		fatalf("%s", err)
 	}
 
+	if err := localClient.CheckReversePathFiltering(context.Background()); err != nil {
+		warnf("%v", err)
+	}
 	if len(prefs.AdvertiseRoutes) > 0 {
 		if err := localClient.CheckIPForwarding(context.Background()); err != nil {
 			warnf("%v", err)

ipn/ipnlocal/local.go

@@ -3450,6 +3450,16 @@ func (b *LocalBackend) CheckIPForwarding() error {
 	return warn
 }
 
+func (b *LocalBackend) CheckRPFilter() error {
+	b.logf("CheckRPFilter called")
+	// TODO: let the caller pass in the ranges, as with CheckIPForwarding above.
+	warn, err := netutil.CheckReversePathFiltering(tsaddr.ExitRoutes(), nil)
+	if err != nil {
+		return err
+	}
+	return warn
+}
+
 // DERPMap returns the current DERPMap in use, or nil if not connected.
 func (b *LocalBackend) DERPMap() *tailcfg.DERPMap {
 	b.mu.Lock()

ipn/localapi/localapi.go

@@ -132,6 +132,8 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		h.serveCheckPrefs(w, r)
 	case "/localapi/v0/check-ip-forwarding":
 		h.serveCheckIPForwarding(w, r)
+	case "/localapi/v0/check-reverse-path-filtering":
+		h.serveCheckRPFilter(w, r)
 	case "/localapi/v0/bugreport":
 		h.serveBugReport(w, r)
 	case "/localapi/v0/file-targets":
@@ -350,6 +352,23 @@ func (h *Handler) serveCheckIPForwarding(w http.ResponseWriter, r *http.Request)
 	})
 }
 
+func (h *Handler) serveCheckRPFilter(w http.ResponseWriter, r *http.Request) {
+	if !h.PermitRead {
+		http.Error(w, "reverse path filtering check access denied", http.StatusForbidden)
+		return
+	}
+	var warning string
+	if err := h.b.CheckRPFilter(); err != nil {
+		warning = err.Error()
+	}
+	w.Header().Set("Content-Type", "application/json")
+	json.NewEncoder(w).Encode(struct {
+		Warning string
+	}{
+		Warning: warning,
+	})
+}
+
 func (h *Handler) serveStatus(w http.ResponseWriter, r *http.Request) {
 	if !h.PermitRead {
 		http.Error(w, "status access denied", http.StatusForbidden)

net/netutil/ip_forward.go

@@ -8,6 +8,7 @@ package netutil
 import (
 	"bytes"
 	"fmt"
+	"log"
 	"net/netip"
 	"os"
 	"os/exec"
@@ -146,6 +147,87 @@ func CheckIPForwarding(routes []netip.Prefix, state *interfaces.State) (warn, 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 state param can be nil, in which case interfaces.GetState is used.
+//
+// The routes should only be advertised routes, and should not contain the
+// nodes Tailscale IPs.
+//
+// This function returns an error if it is unable to determine if reverse path
+// filtering is enabled, or a warning describing configuration issues if
+// reverse path fitering is non-functional or partly functional.
+func CheckReversePathFiltering(routes []netip.Prefix, state *interfaces.State) (warn, err error) {
+	if runtime.GOOS != "linux" {
+		return nil, nil
+	}
+	const kbLink = "" // TODO(andrew): insert one like "\nSee https://tailscale.com/kb/something"
+
+	if state == nil {
+		var err error
+		state, err = interfaces.GetState()
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	// Reverse path filtering as a syscall is only implemented on Linux for IPv4.
+	wantV4, _ := protocolsRequiredForForwarding(routes, state)
+	if !wantV4 {
+		return nil, nil
+	}
+
+	// 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("Couldn't check system's reverse path filtering configuration: %w%s", err, kbLink)
+	}
+
+	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
+	}
+
+	var (
+		warnings []string
+	)
+	for _, iface := range state.Interface {
+		if iface.Name == "lo" {
+			continue
+		}
+
+		iSetting, err := reversePathFilterValueLinux(iface.Name)
+		if err != nil {
+			return nil, fmt.Errorf("Couldn't check system's reverse path filtering configuration: %w%s", err, kbLink)
+		}
+		// Perform the same max() that the kernel does
+		if allSetting > iSetting {
+			iSetting = allSetting
+		}
+		log.Printf("%s: rp_filter=%d", iface.Name, iSetting)
+
+		if iSetting == filtStrict {
+			warnings = append(warnings, fmt.Sprintf("Interface %s has strict reverse-path filtering enabled.", iface.Name))
+		}
+	}
+	if len(warnings) > 0 {
+		return fmt.Errorf("%s\nTailscale may not work correctly.%s", strings.Join(warnings, "\n"), kbLink), nil
+	}
+
+	// All good!
+	return nil, 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`
@@ -177,6 +259,25 @@ 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 (
@@ -219,3 +320,29 @@ 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 `sysctl` 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 sysctl
+		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
+}