health: warn about reverse path filtering and exit nodes

When reverse path filtering is in strict mode on Linux, using an exit
node blocks all network connectivity. This change adds a warning about
this to `tailscale status` and the logs.

Example in `tailscale status`:

```
- not connected to home DERP region 22
- The following issues on your machine will likely make usage of exit nodes impossible: [interface "eth0" has strict reverse-path filtering enabled], please set rp_filter=2 instead of rp_filter=1; see https://github.com/tailscale/tailscale/issues/3310
```

Example in the logs:
```
2024/02/21 21:17:07 health("overall"): error: multiple errors:
	not in map poll
	The following issues on your machine will likely make usage of exit nodes impossible: [interface "eth0" has strict reverse-path filtering enabled], please set rp_filter=2 instead of rp_filter=1; see https://github.com/tailscale/tailscale/issues/3310
```

Updates #3310

Signed-off-by: Anton Tolchanov <anton@tailscale.com>
This commit is contained in:
Anton Tolchanov 2024-01-03 00:23:58 +00:00 committed by Anton Tolchanov
parent 7ef1fb113d
commit 8cc5c51888
6 changed files with 69 additions and 19 deletions

View File

@ -652,6 +652,7 @@ func upWorthyWarning(s string) bool {
return strings.Contains(s, healthmsg.TailscaleSSHOnBut) || return strings.Contains(s, healthmsg.TailscaleSSHOnBut) ||
strings.Contains(s, healthmsg.WarnAcceptRoutesOff) || strings.Contains(s, healthmsg.WarnAcceptRoutesOff) ||
strings.Contains(s, healthmsg.LockedOut) || strings.Contains(s, healthmsg.LockedOut) ||
strings.Contains(s, healthmsg.WarnExitNodeUsage) ||
strings.Contains(strings.ToLower(s), "update available: ") strings.Contains(strings.ToLower(s), "update available: ")
} }

View File

@ -103,6 +103,16 @@ func WithMapDebugFlag(name string) WarnableOpt {
}) })
} }
// WithConnectivityImpact returns an option which makes a Warnable annotated as
// something that could be breaking external network connectivity on the
// machine. This will make the warnable returned by OverallError alongside
// network connectivity errors.
func WithConnectivityImpact() WarnableOpt {
return warnOptFunc(func(w *Warnable) {
w.hasConnectivityImpact = true
})
}
type warnOptFunc func(*Warnable) type warnOptFunc func(*Warnable)
func (f warnOptFunc) mod(w *Warnable) { f(w) } func (f warnOptFunc) mod(w *Warnable) { f(w) }
@ -112,6 +122,10 @@ func (f warnOptFunc) mod(w *Warnable) { f(w) }
type Warnable struct { type Warnable struct {
debugFlag string // optional MapRequest.DebugFlag to send when unhealthy debugFlag string // optional MapRequest.DebugFlag to send when unhealthy
// If true, this warning is related to configuration of networking stack
// on the machine that impacts connectivity.
hasConnectivityImpact bool
isSet atomic.Bool isSet atomic.Bool
mu sync.Mutex mu sync.Mutex
err error err error
@ -442,9 +456,35 @@ func OverallError() error {
var fakeErrForTesting = envknob.RegisterString("TS_DEBUG_FAKE_HEALTH_ERROR") var fakeErrForTesting = envknob.RegisterString("TS_DEBUG_FAKE_HEALTH_ERROR")
// networkErrorf creates an error that indicates issues with outgoing network
// connectivity. Any active warnings related to network connectivity will
// automatically be appended to it.
func networkErrorf(format string, a ...any) error {
errs := []error{
fmt.Errorf(format, a...),
}
for w := range warnables {
if !w.hasConnectivityImpact {
continue
}
if err := w.get(); err != nil {
errs = append(errs, err)
}
}
if len(errs) == 1 {
return errs[0]
}
return multierr.New(errs...)
}
var errNetworkDown = networkErrorf("network down")
var errNotInMapPoll = networkErrorf("not in map poll")
var errNoDERPHome = errors.New("no DERP home")
var errNoUDP4Bind = networkErrorf("no udp4 bind")
func overallErrorLocked() error { func overallErrorLocked() error {
if !anyInterfaceUp { if !anyInterfaceUp {
return errors.New("network down") return errNetworkDown
} }
if localLogConfigErr != nil { if localLogConfigErr != nil {
return localLogConfigErr return localLogConfigErr
@ -457,26 +497,26 @@ func overallErrorLocked() error {
} }
now := time.Now() now := time.Now()
if !inMapPoll && (lastMapPollEndedAt.IsZero() || now.Sub(lastMapPollEndedAt) > 10*time.Second) { if !inMapPoll && (lastMapPollEndedAt.IsZero() || now.Sub(lastMapPollEndedAt) > 10*time.Second) {
return errors.New("not in map poll") return errNotInMapPoll
} }
const tooIdle = 2*time.Minute + 5*time.Second const tooIdle = 2*time.Minute + 5*time.Second
if d := now.Sub(lastStreamedMapResponse).Round(time.Second); d > tooIdle { if d := now.Sub(lastStreamedMapResponse).Round(time.Second); d > tooIdle {
return fmt.Errorf("no map response in %v", d) return networkErrorf("no map response in %v", d)
} }
if !derpHomeless { if !derpHomeless {
rid := derpHomeRegion rid := derpHomeRegion
if rid == 0 { if rid == 0 {
return errors.New("no DERP home") return errNoDERPHome
} }
if !derpRegionConnected[rid] { if !derpRegionConnected[rid] {
return fmt.Errorf("not connected to home DERP region %v", rid) return networkErrorf("not connected to home DERP region %v", rid)
} }
if d := now.Sub(derpRegionLastFrame[rid]).Round(time.Second); d > tooIdle { if d := now.Sub(derpRegionLastFrame[rid]).Round(time.Second); d > tooIdle {
return fmt.Errorf("haven't heard from home DERP region %v in %v", rid, d) return networkErrorf("haven't heard from home DERP region %v in %v", rid, d)
} }
} }
if udp4Unbound { if udp4Unbound {
return errors.New("no udp4 bind") return errNoUDP4Bind
} }
// TODO: use // TODO: use

View File

@ -11,4 +11,5 @@
WarnAcceptRoutesOff = "Some peers are advertising routes but --accept-routes is false" WarnAcceptRoutesOff = "Some peers are advertising routes but --accept-routes is false"
TailscaleSSHOnBut = "Tailscale SSH enabled, but " // + ... something from caller 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" 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"
) )

View File

@ -608,6 +608,7 @@ func (b *LocalBackend) linkChange(delta *netmon.ChangeDelta) {
// If the local network configuration has changed, our filter may // If the local network configuration has changed, our filter may
// need updating to tweak default routes. // need updating to tweak default routes.
b.updateFilterLocked(b.netMap, b.pm.CurrentPrefs()) b.updateFilterLocked(b.netMap, b.pm.CurrentPrefs())
updateExitNodeUsageWarning(b.pm.CurrentPrefs(), delta.New)
if peerAPIListenAsync && b.netMap != nil && b.state == ipn.Running { if peerAPIListenAsync && b.netMap != nil && b.state == ipn.Running {
want := b.netMap.GetAddresses().Len() want := b.netMap.GetAddresses().Len()
@ -3086,6 +3087,22 @@ func (b *LocalBackend) isDefaultServerLocked() bool {
return prefs.ControlURLOrDefault() == ipn.DefaultControlURL return prefs.ControlURLOrDefault() == ipn.DefaultControlURL
} }
var warnExitNodeUsage = health.NewWarnable(health.WithConnectivityImpact())
// updateExitNodeUsageWarning updates a warnable meant to notify users of
// configuration issues that could break exit node usage.
func updateExitNodeUsageWarning(p ipn.PrefsView, state *interfaces.State) {
var result error
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 {
result = fmt.Errorf("%s: %v, %s", healthmsg.WarnExitNodeUsage, warn, comment)
}
}
warnExitNodeUsage.Set(result)
}
func (b *LocalBackend) checkExitNodePrefsLocked(p *ipn.Prefs) error { func (b *LocalBackend) checkExitNodePrefsLocked(p *ipn.Prefs) error {
if (p.ExitNodeIP.IsValid() || p.ExitNodeID != "") && p.AdvertisesExitNode() { if (p.ExitNodeIP.IsValid() || p.ExitNodeID != "") && p.AdvertisesExitNode() {
return errors.New("Cannot advertise an exit node and use an exit node at the same time.") return errors.New("Cannot advertise an exit node and use an exit node at the same time.")

View File

@ -153,7 +153,7 @@ func CheckIPForwarding(routes []netip.Prefix, state *interfaces.State) (warn, er
// This function returns an error if it is unable to determine whether reverse // This function returns an error if it is unable to determine whether reverse
// path filtering is enabled, or a warning describing configuration issues if // path filtering is enabled, or a warning describing configuration issues if
// reverse path fitering is non-functional or partly functional. // reverse path fitering is non-functional or partly functional.
func CheckReversePathFiltering(routes []netip.Prefix, state *interfaces.State) (warn []string, err error) { func CheckReversePathFiltering(state *interfaces.State) (warn []string, err error) {
if runtime.GOOS != "linux" { if runtime.GOOS != "linux" {
return nil, nil return nil, nil
} }
@ -166,12 +166,6 @@ func CheckReversePathFiltering(routes []netip.Prefix, state *interfaces.State) (
} }
} }
// 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' // The kernel uses the maximum value for rp_filter between the 'all'
// setting and each per-interface config, so we need to fetch both. // setting and each per-interface config, so we need to fetch both.
allSetting, err := reversePathFilterValueLinux("all") allSetting, err := reversePathFilterValueLinux("all")
@ -205,7 +199,7 @@ func CheckReversePathFiltering(routes []netip.Prefix, state *interfaces.State) (
iSetting = allSetting iSetting = allSetting
} }
if iSetting == filtStrict { if iSetting == filtStrict {
warn = append(warn, fmt.Sprintf("Interface %q has strict reverse-path filtering enabled", iface.Name)) warn = append(warn, fmt.Sprintf("interface %q has strict reverse-path filtering enabled", iface.Name))
} }
} }
return warn, nil return warn, nil

View File

@ -6,7 +6,6 @@
import ( import (
"io" "io"
"net" "net"
"net/netip"
"runtime" "runtime"
"testing" "testing"
) )
@ -71,9 +70,7 @@ func TestCheckReversePathFiltering(t *testing.T) {
if runtime.GOOS != "linux" { if runtime.GOOS != "linux" {
t.Skipf("skipping on %s", runtime.GOOS) t.Skipf("skipping on %s", runtime.GOOS)
} }
warn, err := CheckReversePathFiltering([]netip.Prefix{ warn, err := CheckReversePathFiltering(nil)
netip.MustParsePrefix("192.168.1.1/24"),
}, nil)
t.Logf("err: %v", err) t.Logf("err: %v", err)
t.Logf("warnings: %v", warn) t.Logf("warnings: %v", warn)
} }