From 4e6a9ffc8cd6107b4e8d75d725293a671ba30e05 Mon Sep 17 00:00:00 2001 From: Jonathan Nobels Date: Thu, 17 Sep 2026 14:04:50 +0000 Subject: [PATCH] feature/exitnodehealth: warn when selected exit nodes are unavailable updates tailscale/corp#33007 When a selected exit node can't carry internet traffic due to a misconfiguration (wrong ID, node deleted, routes removed, etc), blackhole default routes are installed and all internet traffic is dropped. That is the correct behavior -- better to drop than leak to the local network -- but it was entirely silent: the stable-ID lookup in nodeBackend.updateRouteManagerPrefs misses without a log line, and Status leaves ExitNodeStatus nil. The user sees a healthy Tailscale with no internet. If an admin fat-fingers the exit node name in an IT policy, for example, it's easy to break every node with zero feedback. This adds an exit-node-unavailable health warning covering every way the selection can fail to carry traffic (short of reachability which is a separate concern), reported via exitnodehealth.ArgExitNodeReason. The warning names the exit node, caching its display name while it is still a peer so the name survives its departure, and falls back to the stable ID or IP. When the selection is mandated by the ExitNodeID or ExitNodeIP policy settings, the message tells the user to contact their network administrator instead of suggesting they pick another exit node. To test this, set a forced exit node policy with some random ip or node id. The warning has a 5 second threshold. It should clear as soon as you change to a proper exit node. Signed-off-by: Jonathan Nobels --- cmd/k8s-operator/depaware.txt | 1 + cmd/tailscaled/depaware.txt | 1 + cmd/tailscaled/deps_test.go | 13 + cmd/tsidp/depaware.txt | 1 + .../feature_exitnodehealth_disabled.go | 13 + .../feature_exitnodehealth_enabled.go | 13 + feature/condregister/maybe_exitnodehealth.go | 8 + feature/exitnodehealth/exitnodehealth.go | 364 ++++++++++ feature/exitnodehealth/exitnodehealth_test.go | 660 ++++++++++++++++++ feature/featuretags/featuretags.go | 5 + feature/featuretags/featuretags_test.go | 4 + ipn/ipnext/ipnext.go | 20 + ipn/ipnlocal/fortest.go | 36 + ipn/ipnlocal/local.go | 72 +- ipn/ipnlocal/local_test.go | 47 +- ipn/prefs.go | 9 + tsconst/health.go | 1 + tsnet/depaware.txt | 1 + tsnet/maybe_exitnodehealth.go | 8 + 19 files changed, 1248 insertions(+), 29 deletions(-) create mode 100644 feature/buildfeatures/feature_exitnodehealth_disabled.go create mode 100644 feature/buildfeatures/feature_exitnodehealth_enabled.go create mode 100644 feature/condregister/maybe_exitnodehealth.go create mode 100644 feature/exitnodehealth/exitnodehealth.go create mode 100644 feature/exitnodehealth/exitnodehealth_test.go create mode 100644 tsnet/maybe_exitnodehealth.go diff --git a/cmd/k8s-operator/depaware.txt b/cmd/k8s-operator/depaware.txt index c34230ad8..2e2a64ad9 100644 --- a/cmd/k8s-operator/depaware.txt +++ b/cmd/k8s-operator/depaware.txt @@ -742,6 +742,7 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/ tailscale.com/feature/condregister/oauthkey from tailscale.com/tsnet tailscale.com/feature/condregister/portmapper from tailscale.com/tsnet tailscale.com/feature/condregister/useproxy from tailscale.com/tsnet + tailscale.com/feature/exitnodehealth from tailscale.com/tsnet tailscale.com/feature/favorites/pintype from tailscale.com/client/local tailscale.com/feature/netlog from tailscale.com/feature/condregister/netlog tailscale.com/feature/oauthkey from tailscale.com/feature/condregister/oauthkey diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 7d14816b8..570e57eb2 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -316,6 +316,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/feature/dnsresolvecache from tailscale.com/feature/condregister tailscale.com/feature/doctor from tailscale.com/feature/condregister tailscale.com/feature/drive from tailscale.com/feature/condregister + tailscale.com/feature/exitnodehealth from tailscale.com/feature/condregister tailscale.com/feature/favorites from tailscale.com/feature/condregister tailscale.com/feature/favorites/pintype from tailscale.com/client/local+ L tailscale.com/feature/linkspeed from tailscale.com/feature/condregister diff --git a/cmd/tailscaled/deps_test.go b/cmd/tailscaled/deps_test.go index 717cd33f1..627de9162 100644 --- a/cmd/tailscaled/deps_test.go +++ b/cmd/tailscaled/deps_test.go @@ -378,3 +378,16 @@ func TestMinTailscaledWithCLI(t *testing.T) { }, }.Check(t) } + +func TestOmitExitNodeHealth(t *testing.T) { + for _, tag := range []string{"ts_omit_exitnodehealth", "ts_omit_health", "ts_omit_useexitnode"} { + t.Run(tag, func(t *testing.T) { + deptest.DepChecker{ + GOOS: "linux", + GOARCH: "amd64", + Tags: tag + ",ts_include_cli", + BadDeps: map[string]string{"tailscale.com/feature/exitnodehealth": "unexpected exit node health feature"}, + }.Check(t) + }) + } +} diff --git a/cmd/tsidp/depaware.txt b/cmd/tsidp/depaware.txt index c9963200e..d5ae7e403 100644 --- a/cmd/tsidp/depaware.txt +++ b/cmd/tsidp/depaware.txt @@ -152,6 +152,7 @@ tailscale.com/cmd/tsidp dependencies: (generated by github.com/tailscale/depawar tailscale.com/feature/condregister/oauthkey from tailscale.com/tsnet tailscale.com/feature/condregister/portmapper from tailscale.com/tsnet tailscale.com/feature/condregister/useproxy from tailscale.com/tsnet + tailscale.com/feature/exitnodehealth from tailscale.com/tsnet tailscale.com/feature/favorites/pintype from tailscale.com/client/local tailscale.com/feature/netlog from tailscale.com/feature/condregister/netlog tailscale.com/feature/oauthkey from tailscale.com/feature/condregister/oauthkey diff --git a/feature/buildfeatures/feature_exitnodehealth_disabled.go b/feature/buildfeatures/feature_exitnodehealth_disabled.go new file mode 100644 index 000000000..6d664b4fe --- /dev/null +++ b/feature/buildfeatures/feature_exitnodehealth_disabled.go @@ -0,0 +1,13 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +// Code generated by gen.go; DO NOT EDIT. + +//go:build ts_omit_exitnodehealth + +package buildfeatures + +// HasExitNodeHealth is whether the binary was built with support for modular feature "Health warnings for unavailable exit nodes". +// Specifically, it's whether the binary was NOT built with the "ts_omit_exitnodehealth" build tag. +// It's a const so it can be used for dead code elimination. +const HasExitNodeHealth = false diff --git a/feature/buildfeatures/feature_exitnodehealth_enabled.go b/feature/buildfeatures/feature_exitnodehealth_enabled.go new file mode 100644 index 000000000..d0862f974 --- /dev/null +++ b/feature/buildfeatures/feature_exitnodehealth_enabled.go @@ -0,0 +1,13 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +// Code generated by gen.go; DO NOT EDIT. + +//go:build !ts_omit_exitnodehealth + +package buildfeatures + +// HasExitNodeHealth is whether the binary was built with support for modular feature "Health warnings for unavailable exit nodes". +// Specifically, it's whether the binary was NOT built with the "ts_omit_exitnodehealth" build tag. +// It's a const so it can be used for dead code elimination. +const HasExitNodeHealth = true diff --git a/feature/condregister/maybe_exitnodehealth.go b/feature/condregister/maybe_exitnodehealth.go new file mode 100644 index 000000000..aebcfe35d --- /dev/null +++ b/feature/condregister/maybe_exitnodehealth.go @@ -0,0 +1,8 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +//go:build !ts_omit_exitnodehealth && !ts_omit_health && !ts_omit_useexitnode + +package condregister + +import _ "tailscale.com/feature/exitnodehealth" diff --git a/feature/exitnodehealth/exitnodehealth.go b/feature/exitnodehealth/exitnodehealth.go new file mode 100644 index 000000000..9fc37f757 --- /dev/null +++ b/feature/exitnodehealth/exitnodehealth.go @@ -0,0 +1,364 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +// Package exitnodehealth reports unusable exit node configurations via +// health warnables. +// +// It does not infer or probe data-plane reachability. +package exitnodehealth + +import ( + "fmt" + "strings" + "sync" + "time" + + "tailscale.com/feature" + "tailscale.com/feature/buildfeatures" + "tailscale.com/health" + "tailscale.com/ipn" + "tailscale.com/ipn/ipnext" + "tailscale.com/net/tsaddr" + "tailscale.com/tailcfg" + "tailscale.com/tsconst" + "tailscale.com/types/logger" + "tailscale.com/util/syspolicy/pkey" + "tailscale.com/util/syspolicy/policyclient" +) + +const featureName = "exitnodehealth" + +func init() { + if !feature.Register(featureName) { + return + } + ipnext.RegisterExtension(featureName, newExtension) +} + +func newExtension(logf logger.Logf, b ipnext.SafeBackend) (ipnext.Extension, error) { + if !buildfeatures.HasHealth || !buildfeatures.HasUseExitNode { + return nil, ipnext.SkipExtension + } + return &extension{logf: logf, health: b.Sys().HealthTracker.Get(), polc: b.Sys().PolicyClientOrDefault()}, nil +} + +// extension owns the health state for one backend. +type extension struct { + host ipnext.Host + + logf logger.Logf + health *health.Tracker + polc policyclient.Client + + // mu protects the fields below. + // + // Extension callbacks hold the backend mutex before acquiring mu; + // never acquire the backend mutex while holding mu. + mu sync.Mutex + state ipn.State + networkConfigured bool + policyOverridden bool + closed bool + reason ExitNodeHealthVerdict // last reported reason, for transition logs + lastID tailcfg.StableNodeID // last evaluated selection, independent of name caching + + // Remember a peer's name and/or ID so warnings can still identify it after removal. + // It may prove useful to persist this across sessions, but for now we only remember it while the backend is running. + // It is used only for decoration of the health warning. We can always infer the ID or IP from a policy-forced node + // which is the only case where the user cannot fix the problem themselves. + lastKnownID tailcfg.StableNodeID + lastKnownName string +} + +func (*extension) Name() string { return featureName } + +func (e *extension) Init(h ipnext.Host) error { + e.host = h + h.Hooks().BackendStateChange.Add(e.onBackendStateChange) + h.Hooks().ProfileStateChange.Add(e.onProfileStateChange) + h.Hooks().NetworkConfiguredChange.Add(e.onNetworkConfiguredChange) + h.Hooks().OnPeerUpdate.Add(e.onPeerUpdate) + h.Hooks().ExitNodePolicyOverrideChange.Add(e.onPolicyOverrideChange) + return nil +} + +func (e *extension) Shutdown() error { + e.mu.Lock() + defer e.mu.Unlock() + e.closed = true + return nil +} + +func (e *extension) onBackendStateChange(state ipn.State) { + e.mu.Lock() + defer e.mu.Unlock() + e.state = state + e.updateLocked() +} + +func (e *extension) onProfileStateChange(_ ipn.LoginProfileView, _ ipn.PrefsView, sameNode bool) { + e.mu.Lock() + defer e.mu.Unlock() + if !sameNode { + e.lastKnownID, e.lastKnownName = "", "" + } + e.updateLocked() +} + +func (e *extension) onNetworkConfiguredChange(configured bool) { + e.mu.Lock() + defer e.mu.Unlock() + e.networkConfigured = configured + e.updateLocked() +} + +func (e *extension) onPeerUpdate() { + e.mu.Lock() + defer e.mu.Unlock() + e.updateLocked() +} + +func (e *extension) onPolicyOverrideChange(overridden bool) { + e.mu.Lock() + defer e.mu.Unlock() + e.policyOverridden = overridden + e.updateLocked() +} + +// healthContext is the extension's input to warning evaluation. +type healthContext struct { + State ipn.State + NetworkConfigured bool + Prefs ipn.PrefsView + Peer tailcfg.NodeView + PolicyOverridden bool +} + +// updateLocked reads the current selection during an extension callback. +// Both the backend mutex and e.mu are held, so these inputs are consistent. +func (e *extension) updateLocked() { + if e.closed { + return + } + prefs := e.host.Profiles().CurrentPrefs() + node := e.host.NodeBackend() + peer, _ := node.PeerByStableID(prefs.ExitNodeID()) + e.updateWarnableLocked(healthContext{ + State: e.state, + NetworkConfigured: e.networkConfigured, + Prefs: prefs, + Peer: peer, + PolicyOverridden: e.policyOverridden, + }) +} + +// ExitNodeHealthVerdict describes why the selected exit node cannot carry +// internet traffic. It is reported as [ArgExitNodeReason]. +type ExitNodeHealthVerdict string + +const ( + // ExitNodeOK means the exit node configuration is fine: either no exit + // node is selected, or the selected one is a peer offering exit routes. + ExitNodeOK ExitNodeHealthVerdict = "" + + // ExitNodeNotInTailnet means the selected exit node is not among the + // current peers, so it has presumably left the tailnet. + ExitNodeNotInTailnet ExitNodeHealthVerdict = "not-in-tailnet" + + // ExitNodeNoExitRoutes means the selected exit node is a current peer but + // doesn't contribute the default routes, so it either stopped advertising + // them or its routes are not approved. + ExitNodeNoExitRoutes ExitNodeHealthVerdict = "no-exit-routes" + + // ExitNodeNotYetSelected means an exit node is required but none has been + // chosen yet, so blackhole routes remain in place. + ExitNodeNotYetSelected ExitNodeHealthVerdict = "not-yet-selected" +) + +// Bespoke args for exit node health warnables. +const ( + // ArgExitNodeName provides a Warnable with a human-readable identifier for + // the selected exit node: its display name if it is (or recently was) a + // known peer, otherwise its stable node ID or IP address. It is empty if + // no particular exit node has been selected. + ArgExitNodeName health.Arg = "exit-node-name" + + // ArgExitNodeReason provides a Warnable with the reason the selected exit + // node cannot carry internet traffic: "not-in-tailnet", "no-exit-routes", + // or "not-yet-selected". It lets GUIs distinguish the cases without + // parsing the rendered message. + ArgExitNodeReason health.Arg = "exit-node-reason" + + // ArgExitNodePolicyForced is "true" when the selected exit node is + // mandated by the ExitNodeID or ExitNodeIP policy settings, meaning the + // user cannot resolve the problem themselves and should contact their + // network administrator. + ArgExitNodePolicyForced health.Arg = "exit-node-policy-forced" +) + +// exitNodeUnavailableWarnable is a Warnable for when the selected exit node +// cannot carry internet traffic, either because it is no longer part of the +// tailnet, because it isn't offering exit node service, or because an exit +// node is required but none has been selected yet. In all of those cases the +// blackhole routes described on ipn.Prefs.ExitNodeID are installed and +// internet traffic is dropped, which is safe but otherwise silent. +// +// It is distinct from an exit node that is present and selected but which we +// cannot reach; that is a connectivity problem rather than a configuration +// one. +var exitNodeUnavailableWarnable = health.Register(&health.Warnable{ + Code: tsconst.HealthWarnableExitNodeUnavailable, + Title: "Exit node unavailable", + // High severity because this is likely breaking the user's internet connectivity, + // and they need to take action to fix it or report it. + Severity: health.SeverityHigh, + // Don't warn about the exit node when Tailscale is off or the network is + // down; those both imply that we don't know the current exit node selection + // or its status. + DependsOn: []*health.Warnable{health.IPNStateWarnable, health.NetworkStatusWarnable}, + ImpactsConnectivity: true, + // Brief suppression to avoid flashing warnings for transient exit node problems or + // during setup. + TimeToVisible: 5 * time.Second, + Text: warnableText, +}) + +// warnableText renders the message for [exitNodeUnavailableWarnable] +// from its args: what's wrong, what it means, and what to do about it. +func warnableText(args health.Args) string { + var sb strings.Builder + name := args[ArgExitNodeName] + switch ExitNodeHealthVerdict(args[ArgExitNodeReason]) { + case ExitNodeNoExitRoutes: + if name == "" { + sb.WriteString("The selected exit node is not offering exit node service.") + } else { + fmt.Fprintf(&sb, "The selected exit node %q is not offering exit node service.", name) + } + case ExitNodeNotYetSelected: + sb.WriteString("An exit node is required by policy, but no exit node is available to use.") + case ExitNodeNotInTailnet: + if name == "" { + sb.WriteString("The selected exit node is no longer available on your tailnet.") + } else { + fmt.Fprintf(&sb, "The selected exit node %q is no longer available on your tailnet.", name) + } + default: + sb.WriteString("The selected exit node is unavailable.") + } + + sb.WriteString(" Internet traffic is being dropped to avoid leaking it to the local network.") + if args[ArgExitNodePolicyForced] == "true" { + sb.WriteString(" This exit node is required by your network administrator; contact them for help.") + } else { + sb.WriteString(" Select a different exit node, or turn off exit node use.") + } + return sb.String() +} + +// evaluateExitNodeStatus reports a known problem with the selected exit node, +// and a human-readable name for it. +// +// The returned name is the selected exit node's display name if it is a +// current peer, otherwise its stable ID or IP address, or empty if no +// particular exit node has been selected. +func evaluateExitNodeStatus(c healthContext) (ExitNodeHealthVerdict, string) { + prefs := c.Prefs + if !c.NetworkConfigured || !prefs.Valid() || !prefs.WantRunning() || (c.State != ipn.Running && c.State != ipn.Starting) { + // We don't know the peers yet, or aren't routing any traffic at all, + // so there's nothing to warn about. + return ExitNodeOK, "" + } + switch id := prefs.ExitNodeID(); { + case id == ipn.UnresolvedExitNodeID: + return ExitNodeNotYetSelected, "" + case id != "": + peer := c.Peer + if !peer.Valid() { + return ExitNodeNotInTailnet, string(id) + } + if !tsaddr.ContainsExitRoutes(peer.AllowedIPs()) { + return ExitNodeNoExitRoutes, peer.ComputedName() + } + return ExitNodeOK, peer.ComputedName() + case prefs.ExitNodeIP().IsValid(): + // LocalBackend.resolveExitNodeIPLocked clears ExitNodeIP once it + // finds the peer at that address, so a still-set ExitNodeIP means no + // current peer has it. + return ExitNodeNotInTailnet, prefs.ExitNodeIP().String() + } + return ExitNodeOK, "" +} + +// forcedByPolicy reports whether the current exit node selection +// is mandated by the ExitNodeID or ExitNodeIP policy settings, in which case +// the user can't fix an unusable exit node themselves. This affects the +// string we render in the surfaced health warning. +func (e *extension) forcedByPolicy(overridden bool) bool { + if !buildfeatures.HasSystemPolicy || overridden { + return false + } + if v, _ := e.polc.GetString(pkey.ExitNodeID, ""); v != "" { + return true + } + v, _ := e.polc.GetString(pkey.ExitNodeIP, "") + return v != "" +} + +// updateWarnableLocked raises or clears [exitNodeUnavailableWarnable] to +// reflect known problems with the selected exit node. +func (e *extension) updateWarnableLocked(c healthContext) { + // Forget peer names when network configuration is cleared, so a new + // profile cannot inherit the previous profile's warning. + if !c.NetworkConfigured { + e.lastKnownID, e.lastKnownName = "", "" + } + prefs := c.Prefs + reason, name := evaluateExitNodeStatus(c) + id := prefs.ExitNodeID() + + idChanged := id != e.lastID + e.lastID = id + + // Remember the exit node's display name while it is still a peer, so that + // the warning can name it once it disappears and only its stable ID is + // left in the prefs. + if c.NetworkConfigured && c.Peer.Valid() { + if name != "" { + e.lastKnownID, e.lastKnownName = id, name + } + } else if name == string(id) && e.lastKnownID == id && e.lastKnownName != "" { + name = e.lastKnownName + } + + if reason != e.reason || idChanged { + switch { + case reason != ExitNodeOK && name != "": + e.logf("exit node %q is unusable (%s); dropping internet traffic", name, reason) + case reason != ExitNodeOK: + e.logf("selected exit node is unusable (%s); dropping internet traffic", reason) + default: + e.logf("exit node selection is usable again") + } + e.reason = reason + } + + if reason == ExitNodeOK { + e.health.SetHealthy(exitNodeUnavailableWarnable) + return + } + e.health.SetUnhealthy(exitNodeUnavailableWarnable, e.warnableArgs(reason, name, c.PolicyOverridden)) +} + +// warnableArgs builds the [health.Args] describing an unusable +// exit node for [exitNodeUnavailableWarnable]. +func (e *extension) warnableArgs(reason ExitNodeHealthVerdict, name string, overridden bool) health.Args { + args := health.Args{ArgExitNodeReason: string(reason)} + if name != "" { + args[ArgExitNodeName] = name + } + if e.forcedByPolicy(overridden) { + args[ArgExitNodePolicyForced] = "true" + } + return args +} diff --git a/feature/exitnodehealth/exitnodehealth_test.go b/feature/exitnodehealth/exitnodehealth_test.go new file mode 100644 index 000000000..0b3c31590 --- /dev/null +++ b/feature/exitnodehealth/exitnodehealth_test.go @@ -0,0 +1,660 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +package exitnodehealth + +import ( + "errors" + "net/netip" + "strings" + "testing" + "time" + + qt "github.com/frankban/quicktest" + + "tailscale.com/control/controlclient" + "tailscale.com/feature/buildfeatures" + "tailscale.com/health" + "tailscale.com/ipn" + "tailscale.com/ipn/ipnext" + "tailscale.com/ipn/ipnlocal" + "tailscale.com/ipn/ipnlocal/ipnlocaltest" + "tailscale.com/net/tsaddr" + "tailscale.com/tailcfg" + "tailscale.com/tsd" + "tailscale.com/types/key" + "tailscale.com/types/netmap" + "tailscale.com/types/persist" + "tailscale.com/util/syspolicy/pkey" + "tailscale.com/util/syspolicy/policytest" +) + +func extOf(t *testing.T, b *ipnlocal.LocalBackend) *extension { + t.Helper() + c := qt.New(t) + + e, ok := ipnlocal.GetExt[*extension](b) + c.Assert(ok, qt.IsTrue, qt.Commentf("exit node health extension not registered")) + return e +} + +func contextFor(b *ipnlocal.LocalBackend) healthContext { + e, _ := ipnlocal.GetExt[*extension](b) + e.mu.Lock() + configured := e.networkConfigured + e.mu.Unlock() + c := healthContext{State: b.State(), NetworkConfigured: configured, Prefs: b.Prefs()} + for _, peer := range b.ForTest().Peers() { + if peer.StableID() == c.Prefs.ExitNodeID() { + c.Peer = peer + break + } + } + return c +} + +// exitNodeHealthTestNetMap returns a netmap with two peers: "exit1" +// ("my-gateway"), which offers exit routes, and "plain1" ("laptop"), which does +// not. +func exitNodeHealthTestNetMap() *netmap.NetworkMap { + hi := (&tailcfg.Hostinfo{}).View() + nm := &netmap.NetworkMap{ + SelfNode: (&tailcfg.Node{ + ID: 10, + StableID: "self", + Key: key.NewNode().Public(), + Name: "self.example.ts.net.", + Hostinfo: hi, + Addresses: []netip.Prefix{netip.MustParsePrefix("100.64.0.1/32")}, + MachineAuthorized: true, + }).View(), + Peers: []tailcfg.NodeView{ + (&tailcfg.Node{ + ID: 1, + StableID: "exit1", + Key: key.NewNode().Public(), + DiscoKey: key.NewDisco().Public(), + Name: "my-gateway.example.ts.net.", + Hostinfo: hi, + Addresses: []netip.Prefix{netip.MustParsePrefix("100.64.0.2/32")}, + AllowedIPs: append([]netip.Prefix{netip.MustParsePrefix("100.64.0.2/32")}, tsaddr.ExitRoutes()...), + MachineAuthorized: true, + HomeDERP: 1, + }).View(), + (&tailcfg.Node{ + ID: 2, + StableID: "plain1", + Key: key.NewNode().Public(), + DiscoKey: key.NewDisco().Public(), + Name: "laptop.example.ts.net.", + Hostinfo: hi, + Addresses: []netip.Prefix{netip.MustParsePrefix("100.64.0.3/32")}, + AllowedIPs: []netip.Prefix{netip.MustParsePrefix("100.64.0.3/32")}, + MachineAuthorized: true, + HomeDERP: 1, + }).View(), + }, + } + for i, view := range nm.Peers { + peer := view.AsStruct() + peer.InitDisplayNames("example.ts.net") + nm.Peers[i] = peer.View() + } + return nm +} + +// newExitNodeHealthTestBackend returns a backend with +// [exitNodeHealthTestNetMap] installed, ready to run +// auth reconfiguration. If sys is nil, a default one is used. +func newExitNodeHealthTestBackend(t *testing.T, sys *tsd.System) *ipnlocal.LocalBackend { + t.Helper() + c := qt.New(t) + + if !buildfeatures.HasHealth || !buildfeatures.HasUseExitNode { + t.Skip("exit node health dependencies omitted") + } + + var b *ipnlocal.LocalBackend + if sys == nil { + b = ipnlocaltest.NewBackend(t) + } else { + b = ipnlocaltest.NewBackendWithSys(t, sys) + } + + b.ForTest().InitExtensions() + err := b.ForTest().SetPersist(&persist.Persist{}) + c.Assert(err, qt.IsNil) + + b.ForTest().ApplyNetMap(exitNodeHealthTestNetMap()) + b.ForTest().SetPrefs(&ipn.Prefs{WantRunning: true}) + b.ForTest().SetState(ipn.Running) + return b +} + +// TestExitNodeUnavailableWarning tests that selecting an exit node that can't +// carry internet traffic — because it left the tailnet, because it isn't +// offering exit node service, or because none has been chosen yet — raises +// [exitNodeUnavailableWarnable] rather than silently blackholing traffic. +func TestExitNodeUnavailableWarning(t *testing.T) { + tests := []struct { + name string + prefs *ipn.Prefs + wantReason ExitNodeHealthVerdict + wantName string + }{ + { + name: "no-exit-node", + prefs: &ipn.Prefs{WantRunning: true}, + wantReason: ExitNodeOK, + }, + { + name: "good-exit-node-by-id", + prefs: &ipn.Prefs{WantRunning: true, ExitNodeID: "exit1"}, + wantReason: ExitNodeOK, + }, + { + name: "good-exit-node-by-ip", + prefs: &ipn.Prefs{WantRunning: true, ExitNodeIP: netip.MustParseAddr("100.64.0.2")}, + wantReason: ExitNodeOK, + }, + { + name: "id-not-in-tailnet", + prefs: &ipn.Prefs{WantRunning: true, ExitNodeID: "no-such-node"}, + wantReason: ExitNodeNotInTailnet, + wantName: "no-such-node", + }, + { + name: "ip-never-resolved", + prefs: &ipn.Prefs{WantRunning: true, ExitNodeIP: netip.MustParseAddr("100.64.9.9")}, + wantReason: ExitNodeNotInTailnet, + wantName: "100.64.9.9", + }, + { + name: "peer-offers-no-exit-routes", + prefs: &ipn.Prefs{WantRunning: true, ExitNodeID: "plain1"}, + wantReason: ExitNodeNoExitRoutes, + wantName: "laptop", + }, + { + name: "auto-exit-node-not-yet-selected", + prefs: &ipn.Prefs{WantRunning: true, ExitNodeID: "auto:any"}, + wantReason: ExitNodeNotYetSelected, + }, + { + // Tailscale is stopped, so we're not dropping anything and + // health.IPNStateWarnable is the relevant warning. + name: "not-running", + prefs: &ipn.Prefs{WantRunning: false, ExitNodeID: "no-such-node"}, + wantReason: ExitNodeOK, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + b := newExitNodeHealthTestBackend(t, nil) + b.ForTest().SetPrefs(tt.prefs) + b.ForTest().AuthReconfig() + + extOf(t, b).mu.Lock() + gotReason := extOf(t, b).reason + extOf(t, b).mu.Unlock() + if gotReason != tt.wantReason { + t.Errorf("reason = %q, want %q", gotReason, tt.wantReason) + } + + wantUnhealthy := tt.wantReason != ExitNodeOK + if got := b.HealthTracker().IsUnhealthy(exitNodeUnavailableWarnable); got != wantUnhealthy { + t.Errorf("IsUnhealthy = %v, want %v", got, wantUnhealthy) + } + if !wantUnhealthy { + return + } + + _, gotName := evaluateExitNodeStatus(contextFor(b)) + args := extOf(t, b).warnableArgs(gotReason, gotName, false) + if gotName != tt.wantName { + t.Errorf("exit node name = %q, want %q", gotName, tt.wantName) + } + if got := args[ArgExitNodePolicyForced]; got != "" { + t.Errorf("ArgExitNodePolicyForced = %q, want empty without a policy", got) + } + }) + } +} + +// TestExitNodeUnavailableWarningNamesDepartedNode tests that once the selected +// exit node leaves the tailnet, the warning still names it rather than falling +// back to its stable ID. +func TestExitNodeUnavailableWarningNamesDepartedNode(t *testing.T) { + b := newExitNodeHealthTestBackend(t, nil) + nm := exitNodeHealthTestNetMap() + b.ForTest().SetPrefs(&ipn.Prefs{WantRunning: true, ExitNodeID: "exit1"}) + b.ForTest().AuthReconfig() + if b.HealthTracker().IsUnhealthy(exitNodeUnavailableWarnable) { + t.Fatal("warning set while the exit node is present and offering exit routes") + } + + // The exit node leaves the tailnet. + nm.Peers = nm.Peers[1:] + b.ForTest().ApplyNetMap(nm) + b.ForTest().AuthReconfig() + if !b.HealthTracker().IsUnhealthy(exitNodeUnavailableWarnable) { + t.Fatal("warning not set after the exit node left the tailnet") + } + + reason, _ := evaluateExitNodeStatus(contextFor(b)) + extOf(t, b).mu.Lock() + args := extOf(t, b).warnableArgs(reason, extOf(t, b).lastKnownName, false) + extOf(t, b).mu.Unlock() + if reason != ExitNodeNotInTailnet { + t.Errorf("reason = %q, want %q", reason, ExitNodeNotInTailnet) + } + if got := args[ArgExitNodeName]; got != "my-gateway" { + t.Errorf("ArgExitNodeName = %q, want %q", got, "my-my-gateway") + } + + // And it clears once the exit node comes back. + b.ForTest().ApplyNetMap(exitNodeHealthTestNetMap()) + b.ForTest().AuthReconfig() + if b.HealthTracker().IsUnhealthy(exitNodeUnavailableWarnable) { + t.Fatal("warning not cleared after the exit node returned") + } +} + +// TestExitNodeUnavailableWarningOnNetmapDelta tests the scenario the warning +// exists for: the selected exit node is removed from the tailnet via an +// incremental netmap update, which is the path a real client takes. The +// warning must be raised without anyone calling authReconfig by hand. +func TestExitNodeUnavailableWarningOnNetmapDelta(t *testing.T) { + c := qt.New(t) + + b := newExitNodeHealthTestBackend(t, nil) + b.ForTest().SetPrefs(&ipn.Prefs{WantRunning: true, ExitNodeID: "exit1"}) + b.ForTest().AuthReconfig() + isUnhealthy := b.HealthTracker().IsUnhealthy(exitNodeUnavailableWarnable) + c.Assert(isUnhealthy, qt.IsFalse, qt.Commentf("warning set while the exit node is present and offering exit routes")) + // Control removes the exit node (node ID 1) from the tailnet. + muts, ok := netmap.MutationsFromMapResponse(&tailcfg.MapResponse{ + PeersRemoved: []tailcfg.NodeID{1}, + }, time.Unix(123, 0)) + + c.Assert(ok, qt.IsTrue, qt.Commentf("netmap.MutationsFromMapResponse failed")) + c.Assert(b.UpdateNetmapDelta(muts), qt.IsTrue, qt.Commentf("UpdateNetmapDelta returned false")) + c.Assert(b.HealthTracker().IsUnhealthy(exitNodeUnavailableWarnable), qt.IsTrue, qt.Commentf("warning not set after the exit node was removed by a netmap delta")) + + extOf(t, b).mu.Lock() + gotReason := extOf(t, b).reason + extOf(t, b).mu.Unlock() + if gotReason != ExitNodeNotInTailnet { + t.Errorf("reason = %q, want %q", gotReason, ExitNodeNotInTailnet) + } +} + +// TestExitNodeUnavailableWarningPolicyForced tests that an exit node mandated +// by the ExitNodeID policy setting produces a warning telling the user to +// contact their administrator, since they can't change the selection. +func TestExitNodeUnavailableWarningPolicyForced(t *testing.T) { + if !buildfeatures.HasSystemPolicy { + t.Skip("system policy omitted") + } + + sys := tsd.NewSystem() + sys.PolicyClient.Set(policytest.Config{pkey.ExitNodeID: "no-such-node"}) + b := newExitNodeHealthTestBackend(t, sys) + b.ForTest().SetPrefs(&ipn.Prefs{WantRunning: true}) + b.ForTest().AuthReconfig() + + if !b.HealthTracker().IsUnhealthy(exitNodeUnavailableWarnable) { + t.Fatal("warning not set for a policy-forced exit node that isn't in the tailnet") + } + if got := b.Prefs().ExitNodeID(); got != "no-such-node" { + t.Fatalf("ExitNodeID = %q; policy did not take effect", got) + } + reason, name := evaluateExitNodeStatus(contextFor(b)) + args := extOf(t, b).warnableArgs(reason, name, false) + + if got := args[ArgExitNodePolicyForced]; got != "true" { + t.Errorf("ArgExitNodePolicyForced = %q, want %q", got, "true") + } + if text := warnableText(args); !strings.Contains(text, "network administrator") { + t.Errorf("text = %q; want it to mention the network administrator", text) + } +} + +func TestExitNodeUnavailableText(t *testing.T) { + tests := []struct { + name string + args health.Args + want string + }{ + { + name: "not-in-tailnet", + args: health.Args{ + ArgExitNodeReason: string(ExitNodeNotInTailnet), + ArgExitNodeName: "my-vps", + }, + want: `The selected exit node "my-vps" is no longer available on your tailnet. ` + + "Internet traffic is being dropped to avoid leaking it to the local network. " + + "Select a different exit node, or turn off exit node use.", + }, + { + name: "no-exit-routes", + args: health.Args{ + ArgExitNodeReason: string(ExitNodeNoExitRoutes), + ArgExitNodeName: "laptop", + }, + want: `The selected exit node "laptop" is not offering exit node service. ` + + "Internet traffic is being dropped to avoid leaking it to the local network. " + + "Select a different exit node, or turn off exit node use.", + }, + { + name: "not-yet-selected", + args: health.Args{ArgExitNodeReason: string(ExitNodeNotYetSelected)}, + want: "An exit node is required by policy, but no exit node is available to use. " + + "Internet traffic is being dropped to avoid leaking it to the local network. " + + "Select a different exit node, or turn off exit node use.", + }, + { + name: "policy-forced", + args: health.Args{ + ArgExitNodeReason: string(ExitNodeNotInTailnet), + ArgExitNodeName: "corp-exit", + ArgExitNodePolicyForced: "true", + }, + want: `The selected exit node "corp-exit" is no longer available on your tailnet. ` + + "Internet traffic is being dropped to avoid leaking it to the local network. " + + "This exit node is required by your network administrator; contact them for help.", + }, + { + name: "unnamed", + args: health.Args{ArgExitNodeReason: string(ExitNodeNotInTailnet)}, + want: "The selected exit node is no longer available on your tailnet. " + + "Internet traffic is being dropped to avoid leaking it to the local network. " + + "Select a different exit node, or turn off exit node use.", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := warnableText(tt.args); got != tt.want { + t.Errorf("exitNodeUnavailableText() =\n %q\nwant\n %q", got, tt.want) + } + }) + } +} + +func TestWarningClears(t *testing.T) { + tests := []struct { + name string + stop bool + }{ + {name: "no-netmap"}, + {name: "stopped", stop: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + b := newExitNodeHealthTestBackend(t, nil) + b.ForTest().SetPrefs(&ipn.Prefs{WantRunning: true, ExitNodeID: "missing"}) + b.ForTest().AuthReconfig() + if !b.HealthTracker().IsUnhealthy(exitNodeUnavailableWarnable) { + t.Fatal("warning not raised") + } + + if tt.stop { + b.ForTest().SetPrefs(&ipn.Prefs{ExitNodeID: "missing"}) + } else { + b.ForTest().ApplyNetMap(nil) + } + b.ForTest().AuthReconfig() + if b.HealthTracker().IsUnhealthy(exitNodeUnavailableWarnable) { + t.Fatal("warning not cleared") + } + }) + } +} + +func TestPolicyArgs(t *testing.T) { + tests := []struct { + name string + policyKey pkey.Key + overridden bool + }{ + {name: "exit-node-id", policyKey: pkey.ExitNodeID}, + {name: "exit-node-id-overridden", policyKey: pkey.ExitNodeID, overridden: true}, + {name: "exit-node-ip", policyKey: pkey.ExitNodeIP}, + {name: "exit-node-ip-overridden", policyKey: pkey.ExitNodeIP, overridden: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + e := &extension{polc: policytest.Config{tt.policyKey: "configured"}} + args := e.warnableArgs(ExitNodeNotInTailnet, "missing", tt.overridden) + + want := buildfeatures.HasSystemPolicy && !tt.overridden + if got := args[ArgExitNodePolicyForced] == "true"; got != want { + t.Errorf("policy forced = %v, want %v", got, want) + } + }) + } +} + +func TestLogsOnlyTransitions(t *testing.T) { + var logs []string + e := &extension{polc: policytest.Config{}, logf: func(format string, args ...any) { + logs = append(logs, format) + }} + // A nil health tracker supports warning updates as no-ops. + c := healthContext{State: ipn.Running, NetworkConfigured: true, Prefs: (&ipn.Prefs{WantRunning: true, ExitNodeID: "missing"}).View()} + e.updateWarnableLocked(c) + e.updateWarnableLocked(c) + c.NetworkConfigured = false + e.updateWarnableLocked(c) + e.updateWarnableLocked(c) + if len(logs) != 2 { + t.Errorf("got %d logs, want two transitions", len(logs)) + } +} + +func TestMissingDependencies(t *testing.T) { + if buildfeatures.HasHealth && buildfeatures.HasUseExitNode { + t.Skip("all dependencies included") + } + // Skipping must happen before the constructor accesses the backend. + if _, err := newExtension(t.Logf, nil); !errors.Is(err, ipnext.SkipExtension) { + t.Fatalf("newExtension = %v, want SkipExtension", err) + } +} + +func wantReason(t *testing.T, b *ipnlocal.LocalBackend, want ExitNodeHealthVerdict) { + t.Helper() + e := extOf(t, b) + e.mu.Lock() + got := e.reason + e.mu.Unlock() + if got != want { + t.Errorf("reason = %q, want %q", got, want) + } + if got := b.HealthTracker().IsUnhealthy(exitNodeUnavailableWarnable); got != (want != ExitNodeOK) { + t.Errorf("warning raised = %v, want %v", got, want != ExitNodeOK) + } +} + +func TestStateChangesWithoutReconfig(t *testing.T) { + tests := []struct { + name string + state ipn.State + }{ + {name: "no-state", state: ipn.NoState}, + {name: "stopped", state: ipn.Stopped}, + {name: "needs-login", state: ipn.NeedsLogin}, + {name: "needs-machine-auth", state: ipn.NeedsMachineAuth}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + b := newExitNodeHealthTestBackend(t, nil) + b.ForTest().SetPrefs(&ipn.Prefs{WantRunning: true, ExitNodeID: "missing"}) + wantReason(t, b, ExitNodeNotInTailnet) + + b.ForTest().SetState(tt.state) + wantReason(t, b, ExitNodeOK) + + // WantRunning and the selected exit node have not changed. + b.ForTest().SetState(ipn.Running) + wantReason(t, b, ExitNodeNotInTailnet) + }) + } +} + +func TestFullNetmapChangesWithoutReconfig(t *testing.T) { + tests := []struct { + name string + netmap func() *netmap.NetworkMap + wantReason ExitNodeHealthVerdict + }{ + { + name: "exit-node-offline", + netmap: func() *netmap.NetworkMap { + nm := exitNodeHealthTestNetMap() + peer := nm.Peers[0].AsStruct() + peer.Online = new(false) + nm.Peers[0] = peer.View() + return nm + }, + wantReason: ExitNodeOK, + }, + { + name: "exit-node-without-exit-routes", + netmap: func() *netmap.NetworkMap { + nm := exitNodeHealthTestNetMap() + peer := nm.Peers[0].AsStruct() + peer.AllowedIPs = nil + nm.Peers[0] = peer.View() + return nm + }, + wantReason: ExitNodeNoExitRoutes, + }, + { + name: "no-netmap", + netmap: func() *netmap.NetworkMap { return nil }, + wantReason: ExitNodeOK, + }, + } + b := newExitNodeHealthTestBackend(t, nil) + b.ForTest().SetPrefs(&ipn.Prefs{WantRunning: true, ExitNodeID: "exit1"}) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + b.ForTest().ApplyNetMap(tt.netmap()) + wantReason(t, b, tt.wantReason) + }) + } +} + +func TestProfileChangeClearsWarningAndName(t *testing.T) { + b := newExitNodeHealthTestBackend(t, nil) + b.ForTest().SetPrefs(&ipn.Prefs{WantRunning: true, ExitNodeID: "exit1"}) + nm := exitNodeHealthTestNetMap() + nm.Peers = nm.Peers[1:] + b.ForTest().ApplyNetMap(nm) + wantReason(t, b, ExitNodeNotInTailnet) + e := extOf(t, b) + e.mu.Lock() + name := e.lastKnownName + e.mu.Unlock() + if name != "my-gateway" { + t.Fatalf("remembered name = %q, want my-gateway", name) + } + + // Exercise the real profile reset, but stop before starting a control client. + errNoClient := errors.New("test: no control client") + b.ForTest().SetControlClientGetter(func(controlclient.Options) (controlclient.Client, error) { return nil, errNoClient }) + if err := b.NewProfile(); !errors.Is(err, errNoClient) { + t.Fatalf("NewProfile = %v, want %v", err, errNoClient) + } + wantReason(t, b, ExitNodeOK) + e.mu.Lock() + defer e.mu.Unlock() + if e.lastKnownID != "" || e.lastKnownName != "" { + t.Errorf("profile reset retained %q / %q", e.lastKnownID, e.lastKnownName) + } +} + +// Route updates must use the live peer map, not the original full netmap. +func TestExitNodeRouteChangesOnNetmapDelta(t *testing.T) { + b := newExitNodeHealthTestBackend(t, nil) + b.ForTest().SetPrefs(&ipn.Prefs{WantRunning: true, ExitNodeID: "exit1"}) + node, _ := b.NodeBackend().PeerByStableID("exit1") + peer := node.AsStruct() + for _, offerRoutes := range []bool{false, true} { + peer.AllowedIPs = nil + want := ExitNodeNoExitRoutes + if offerRoutes { + peer.AllowedIPs = tsaddr.ExitRoutes() + want = ExitNodeOK + } + muts, ok := netmap.MutationsFromMapResponse(&tailcfg.MapResponse{ + PeersChanged: []*tailcfg.Node{peer.Clone()}, + }, time.Unix(123, 0)) + if !ok || !b.UpdateNetmapDelta(muts) { + t.Fatal("failed to apply peer route update") + } + wantReason(t, b, want) + } +} + +func TestPolicyOverrideEvents(t *testing.T) { + if !buildfeatures.HasSystemPolicy { + t.Skip("system policy omitted") + } + sys := tsd.NewSystem() + sys.PolicyClient.Set(policytest.Config{ + pkey.ExitNodeID: "missing", + pkey.AllowExitNodeOverride: true, + }) + b := newExitNodeHealthTestBackend(t, sys) + wantReason(t, b, ExitNodeNotInTailnet) + + checkOverride := func(want bool) { + t.Helper() + e := extOf(t, b) + e.mu.Lock() + defer e.mu.Unlock() + if e.policyOverridden != want { + t.Errorf("policyOverridden = %v, want %v", e.policyOverridden, want) + } + if got := e.forcedByPolicy(e.policyOverridden); got != !want { + t.Errorf("forcedByPolicy = %v, want %v", got, !want) + } + } + checkOverride(false) + if _, err := b.EditPrefs(&ipn.MaskedPrefs{ + ExitNodeIDSet: true, + Prefs: ipn.Prefs{ExitNodeID: "exit1"}, + }); err != nil { + t.Fatal(err) + } + checkOverride(true) + + // Disconnecting resets the override, even without changing the selection. + if _, err := b.EditPrefs(&ipn.MaskedPrefs{WantRunningSet: true}); err != nil { + t.Fatal(err) + } + checkOverride(false) + wantReason(t, b, ExitNodeOK) +} + +// An empty peer list is evidence of a missing exit node only after the node +// has received network configuration. Clearing that configuration must stop +// evaluation, and receiving it again must resume evaluation. +func TestNetworkConfigurationGatesDetection(t *testing.T) { + b := newExitNodeHealthTestBackend(t, nil) + b.ForTest().ApplyNetMap(nil) + b.ForTest().SetPrefs(&ipn.Prefs{WantRunning: true, ExitNodeID: "missing"}) + wantReason(t, b, ExitNodeOK) + + nm := exitNodeHealthTestNetMap() + nm.Peers = nil + for range 2 { + b.ForTest().ApplyNetMap(nm) + wantReason(t, b, ExitNodeNotInTailnet) + b.ForTest().ApplyNetMap(nil) + wantReason(t, b, ExitNodeOK) + } +} diff --git a/feature/featuretags/featuretags.go b/feature/featuretags/featuretags.go index 069f9413d..b2030f107 100644 --- a/feature/featuretags/featuretags.go +++ b/feature/featuretags/featuretags.go @@ -309,6 +309,11 @@ type FeatureMeta struct { Sym: "UseRoutes", Desc: "Use routes advertised by other nodes", }, + "exitnodehealth": { + Sym: "ExitNodeHealth", + Desc: "Health warnings for unavailable exit nodes", + Deps: []FeatureTag{"health", "useexitnode"}, + }, "useexitnode": { Sym: "UseExitNode", Desc: "Use exit nodes", diff --git a/feature/featuretags/featuretags_test.go b/feature/featuretags/featuretags_test.go index c8a9f77ae..c0c406fd0 100644 --- a/feature/featuretags/featuretags_test.go +++ b/feature/featuretags/featuretags_test.go @@ -35,6 +35,10 @@ func TestRequires(t *testing.T) { in FeatureTag want set.Set[FeatureTag] }{ + { + in: "exitnodehealth", + want: setOf("exitnodehealth", "health", "useexitnode", "peerapiclient", "useroutes"), + }, { in: "drive", want: setOf("drive"), diff --git a/ipn/ipnext/ipnext.go b/ipn/ipnext/ipnext.go index 32565b2c0..e651a813c 100644 --- a/ipn/ipnext/ipnext.go +++ b/ipn/ipnext/ipnext.go @@ -418,6 +418,23 @@ type Hooks struct { // or when the client disconnects and the network map is cleared. OnNetMapToggle feature.Hooks[func(*netmap.NetworkMap)] + // NetworkConfiguredChange is called with LocalBackend.mu held when the + // current node receives its initial network configuration or that + // configuration is cleared, including during a profile reset. + NetworkConfiguredChange feature.Hooks[func(configured bool)] + + // OnPeerUpdate is called with LocalBackend.mu held after processing a + // replacement, incremental update, or clear of the current node's peers. + // The peer state need not differ from its previous value. + // Callbacks can query [Host.NodeBackend] for the current peers. + // It runs independently of engine reconfiguration. + OnPeerUpdate feature.Hooks[func()] + + // ExitNodePolicyOverrideChange is called with LocalBackend.mu held when + // the exit node policy override is set or reset. It may also be called + // with an unchanged value when the underlying policy changes. + ExitNodePolicyOverrideChange feature.Hooks[func(overridden bool)] + // OnSelfChange is called (with LocalBackend.mu held) when the self node // changes, including changing to nothing (an invalid view). OnSelfChange feature.Hooks[func(tailcfg.NodeView)] @@ -520,6 +537,9 @@ type FilterHooks struct { // // It is not a snapshot in time but is locked to a particular node. type NodeBackend interface { + // PeerByStableID returns a current peer, including incremental updates. + PeerByStableID(tailcfg.StableNodeID) (tailcfg.NodeView, bool) + // Self returns the current node. Self() tailcfg.NodeView diff --git a/ipn/ipnlocal/fortest.go b/ipn/ipnlocal/fortest.go index f820dca09..abfae63e9 100644 --- a/ipn/ipnlocal/fortest.go +++ b/ipn/ipnlocal/fortest.go @@ -14,6 +14,7 @@ "tailscale.com/tstime" "tailscale.com/types/key" "tailscale.com/types/netmap" + "tailscale.com/types/persist" "tailscale.com/util/testenv" "tailscale.com/wgengine/filter" ) @@ -126,6 +127,13 @@ func (f forTest) SetServeConfig(sc ipn.ServeConfigView) { b.serveConfig = sc } +// InitExtensions initializes the extensions without starting a control client. +func (f forTest) InitExtensions() { + f.b.mu.Lock() + defer f.b.mu.Unlock() + f.b.startOnce.Do(f.b.initOnce) +} + // SetNetMap installs nm as the backend's current netmap without going // through control-plane plumbing. It is intended for tests that need a // specific netmap (e.g. CertDomains, capabilities). @@ -150,3 +158,31 @@ func (f forTest) SetPrefs(newp *ipn.Prefs) { defer b.mu.Unlock() b.setPrefsLocked(newp) } + +// AuthReconfig applies the current network map and preferences to the engine. +func (f forTest) AuthReconfig() { f.b.authReconfig() } + +// SetPersist seeds the current profile's persisted identity without running +// the control client or backend state machine. +func (f forTest) SetPersist(p *persist.Persist) error { + b := f.b + b.mu.Lock() + defer b.mu.Unlock() + prefs := b.pm.CurrentPrefs().AsStruct() + prefs.Persist = p.Clone() + return b.pm.SetPrefs(prefs.View(), ipn.NetworkProfile{}) +} + +// SetState changes the backend state and dispatches its state change hooks. +func (f forTest) SetState(state ipn.State) { + f.b.mu.Lock() + defer f.b.mu.Unlock() + f.b.setStateLocked(state) +} + +// ApplyNetMap installs a map through the production path, including feature hooks. +func (f forTest) ApplyNetMap(nm *netmap.NetworkMap) { + f.b.mu.Lock() + defer f.b.mu.Unlock() + f.b.setNetMapLocked(nm) +} diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index ff9c84843..e73a88b51 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -461,6 +461,7 @@ type LocalBackend struct { // or when switching profiles, connecting/disconnecting Tailscale, restarting the client, // or on similar events. // + // Set through setExitNodePolicyOverrideLocked so extensions are notified. // See tailscale/corp#29969. overrideExitNodePolicy bool @@ -2287,7 +2288,7 @@ func (b *LocalBackend) applyExitNodeSysPolicyLocked(prefs *ipn.Prefs) (anyChange // older clients (in case a user downgrades to an earlier version) // and GUIs/CLIs that have special handling for it. if useAutoExitNode { - exitNodeID = unresolvedExitNodeID + exitNodeID = ipn.UnresolvedExitNodeID } // If the current exit node ID doesn't match the one enforced by the policy setting, @@ -2370,7 +2371,7 @@ func (b *LocalBackend) sysPolicyChanged(policy policyclient.PolicyChange) { // Reset the exit node override if a policy that enforces exit node usage // or allows the user to override automatic exit node selection has changed. b.mu.Lock() - b.overrideExitNodePolicy = false + b.setExitNodePolicyOverrideLocked(false) b.mu.Unlock() } @@ -2444,6 +2445,7 @@ func (b *LocalBackend) UpdateNetmapDelta(muts []netmap.NodeMutation) (handled bo needsAuthReconfig := netmapDeltaNeedsAuthReconfig(cn, muts) deltaRes, _ := cn.UpdateNetmapDelta(muts) + b.notifyPeerUpdateLocked() if buildfeatures.HasDrive { // Drive's lazy remotes-source caches its rebuild keyed by this // generation, so any delta — peer add/remove, address change, @@ -2820,10 +2822,10 @@ func (b *LocalBackend) resolveAutoExitNodeLocked(prefs *ipn.Prefs) (prefsChanged // specify an allowed auto exit node ID, retain it. newExitNodeID = prefs.ExitNodeID } else { - // Otherwise, use [unresolvedExitNodeID] to install a blackhole route, + // Otherwise, use [ipn.UnresolvedExitNodeID] to install a blackhole route, // preventing traffic from leaking to the local network until an actual // exit node is selected. - newExitNodeID = unresolvedExitNodeID + newExitNodeID = ipn.UnresolvedExitNodeID } if prefs.ExitNodeID != newExitNodeID { prefs.ExitNodeID = newExitNodeID @@ -5129,7 +5131,7 @@ func (b *LocalBackend) SetUseExitNodeEnabled(actor ipnauth.Actor, v bool) (ipn.P if expr, ok := ipn.ParseAutoExitNodeString(mp.ExitNodeID); ok { mp.AutoExitNodeSet = true mp.AutoExitNode = expr - mp.ExitNodeID = unresolvedExitNodeID + mp.ExitNodeID = ipn.UnresolvedExitNodeID } } else { mp.ExitNodeIDSet = true @@ -5288,7 +5290,7 @@ func (b *LocalBackend) adjustEditPrefsLocked(prefs ipn.PrefsView, mp *ipn.Masked } // Clear ExitNodeID if AutoExitNode is disabled and ExitNodeID is still unresolved. - if mp.AutoExitNodeSet && mp.AutoExitNode == "" && prefs.ExitNodeID() == unresolvedExitNodeID { + if mp.AutoExitNodeSet && mp.AutoExitNode == "" && prefs.ExitNodeID() == ipn.UnresolvedExitNodeID { mp.ExitNodeIDSet = true mp.ExitNodeID = "" } @@ -5328,16 +5330,16 @@ func (b *LocalBackend) onEditPrefsLocked(_ ipnauth.Actor, mp *ipn.MaskedPrefs, o if oldPrefs.WantRunning() != newPrefs.WantRunning() { // Connecting to or disconnecting from Tailscale clears the override, // unless the user is also explicitly changing the exit node (see below). - b.overrideExitNodePolicy = false + b.setExitNodePolicyOverrideLocked(false) } if mp.AutoExitNodeSet || mp.ExitNodeIDSet || mp.ExitNodeIPSet { if allowExitNodeOverride, _ := b.polc.GetBoolean(pkey.AllowExitNodeOverride, false); allowExitNodeOverride { // If applying exit node policy settings to the new prefs results in no change, // the user is not overriding the policy. Otherwise, it is an override. - b.overrideExitNodePolicy = b.applyExitNodeSysPolicyLocked(newPrefs.AsStruct()) + b.setExitNodePolicyOverrideLocked(b.applyExitNodeSysPolicyLocked(newPrefs.AsStruct())) } else { // Overrides are not allowed; clear the override flag. - b.overrideExitNodePolicy = false + b.setExitNodePolicyOverrideLocked(false) } } @@ -6717,7 +6719,7 @@ func (b *LocalBackend) applyPrefsToHostinfoLocked(hi *tailcfg.Hostinfo, prefs ip // [pkey.ExitNodeID]), or an exit node is specified by ExitNodeIP // instead of ExitNodeID , and we don't yet have enough info to resolve // it (usually due to missing netmap or net report), then ExitNodeID in - // the prefs may be invalid (typically, [unresolvedExitNodeID]) until + // the prefs may be invalid (typically, [ipn.UnresolvedExitNodeID]) until // the netmap is available. // // In this case, we shouldn't update the Hostinfo with the bogus @@ -6725,7 +6727,7 @@ func (b *LocalBackend) applyPrefsToHostinfoLocked(hi *tailcfg.Hostinfo, prefs ip // the netmap and/or net report have been received to both pick the exit // node and notify control of the change. if buildfeatures.HasUseExitNode { - if sid := prefs.ExitNodeID(); sid != unresolvedExitNodeID { + if sid := prefs.ExitNodeID(); sid != ipn.UnresolvedExitNodeID { hi.ExitNodeID = prefs.ExitNodeID() } } @@ -7230,7 +7232,7 @@ func (b *LocalBackend) resolveExitNodeLocked() (changed bool) { // TODO(sfllaw): Mutating b.hostinfo here is undesirable, mutating // in-place doubly so. sid := prefs.ExitNodeID - if sid != unresolvedExitNodeID && b.hostinfo.ExitNodeID != sid { + if sid != ipn.UnresolvedExitNodeID && b.hostinfo.ExitNodeID != sid { b.hostinfo.ExitNodeID = sid b.goTracker.Go(b.doSetHostinfoFilterServices) } @@ -7310,6 +7312,14 @@ func (b *LocalBackend) setNetMapLocked(nm *netmap.NetworkMap) { login = cmp.Or(profileFromView(nm.UserProfiles[nm.User()]).LoginName, "") } discoChanged, routeChanged := b.currentNode().SetNetMap(nm) + // A profile reset swaps in a fresh nodeBackend before clearing its map, + // so notify on every clear even if this node never received a map. + if !b.shutdownCalled && (oldNetMap == nil || nm == nil) { + for _, f := range b.extHost.Hooks().NetworkConfiguredChange { + f(nm != nil) + } + } + b.notifyPeerUpdateLocked() b.setDataPlanePeerRoutes() if ms, ok := b.sys.MagicSock.GetOK(); ok { if nm != nil { @@ -7485,6 +7495,31 @@ func (b *LocalBackend) setNetMapLocked(nm *netmap.NetworkMap) { // update. var hookInstallDriveRemoteSource feature.Hook[func(*LocalBackend)] +// notifyPeerUpdateLocked notifies extensions after processing a peer update, +// even if the peer state did not change. +// b.mu must be held. +func (b *LocalBackend) notifyPeerUpdateLocked() { + if b.shutdownCalled { + return + } + for _, f := range b.extHost.Hooks().OnPeerUpdate { + f() + } +} + +// setExitNodePolicyOverrideLocked sets the override and notifies extensions. +// Notify even if the value is unchanged: the policy itself may have changed. +// b.mu must be held. +func (b *LocalBackend) setExitNodePolicyOverrideLocked(overridden bool) { + b.overrideExitNodePolicy = overridden + if b.shutdownCalled { + return + } + for _, f := range b.extHost.Hooks().ExitNodePolicyOverrideChange { + f(overridden) + } +} + // roundTraffic rounds bytes. This is used to preserve user privacy within logs. func roundTraffic(bytes int64) float64 { var x float64 @@ -8372,7 +8407,7 @@ func (b *LocalBackend) resetForProfileChangeLocked() error { b.serveConfig = ipn.ServeConfigView{} b.lastSuggestedExitNode = "" b.keyExpired = false - b.overrideExitNodePolicy = false + b.setExitNodePolicyOverrideLocked(false) b.resetAlwaysOnOverrideLocked() b.extHost.NotifyProfileChange(b.pm.CurrentProfile(), b.pm.CurrentPrefs(), false) b.setAtomicValuesFromPrefsLocked(b.pm.CurrentPrefs()) @@ -9115,17 +9150,6 @@ func longLatDistance(fromLat, fromLong, toLat, toLong float64) float64 { return earthRadiusMeters * c } -const ( - // unresolvedExitNodeID is a special [tailcfg.StableNodeID] value - // used as an exit node ID to install a blackhole route, preventing - // accidental non-exit-node usage until the [ipn.ExitNodeExpression] - // is evaluated and an actual exit node is selected. - // - // We use "auto:any" for compatibility with older, pre-[ipn.ExitNodeExpression] - // clients that have been using "auto:any" for this purpose for a long time. - unresolvedExitNodeID tailcfg.StableNodeID = "auto:any" -) - func isAllowedAutoExitNodeID(polc policyclient.Client, exitNodeID tailcfg.StableNodeID) bool { if exitNodeID == "" { return false // an exit node is required diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index c4379cdbc..35f9d54c7 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -847,7 +847,7 @@ func TestConfigureExitNode(t *testing.T) { }, wantPrefs: ipn.Prefs{ ControlURL: controlURL, - ExitNodeID: unresolvedExitNodeID, // cannot resolve; traffic will be dropped + ExitNodeID: ipn.UnresolvedExitNodeID, // cannot resolve; traffic will be dropped AutoExitNode: "any", }, wantHostinfoExitNodeID: "", @@ -864,7 +864,7 @@ func TestConfigureExitNode(t *testing.T) { }, wantPrefs: ipn.Prefs{ ControlURL: controlURL, - ExitNodeID: unresolvedExitNodeID, // cannot resolve; traffic will be dropped + ExitNodeID: ipn.UnresolvedExitNodeID, // cannot resolve; traffic will be dropped AutoExitNode: "any", }, wantHostinfoExitNodeID: "", @@ -1035,7 +1035,7 @@ func TestConfigureExitNode(t *testing.T) { exitNodeIDPolicy: new(tailcfg.StableNodeID("auto:any")), wantPrefs: ipn.Prefs{ ControlURL: controlURL, - ExitNodeID: unresolvedExitNodeID, + ExitNodeID: ipn.UnresolvedExitNodeID, AutoExitNode: "any", }, wantHostinfoExitNodeID: "", @@ -1050,7 +1050,7 @@ func TestConfigureExitNode(t *testing.T) { exitNodeIDPolicy: new(tailcfg.StableNodeID("auto:any")), wantPrefs: ipn.Prefs{ ControlURL: controlURL, - ExitNodeID: unresolvedExitNodeID, + ExitNodeID: ipn.UnresolvedExitNodeID, AutoExitNode: "any", }, wantHostinfoExitNodeID: "", @@ -1105,7 +1105,7 @@ func TestConfigureExitNode(t *testing.T) { }, wantPrefs: ipn.Prefs{ ControlURL: controlURL, - ExitNodeID: unresolvedExitNodeID, // we don't have a netmap yet, and the current exit node ID is not allowed; block traffic + ExitNodeID: ipn.UnresolvedExitNodeID, // we don't have a netmap yet, and the current exit node ID is not allowed; block traffic AutoExitNode: "any", }, wantHostinfoExitNodeID: "", @@ -10017,3 +10017,40 @@ func TestApplyPrefsToHostinfoDedup(t *testing.T) { }) } } + +// Engine updates being blocked must not suppress exit node health evaluation. +func TestExtensionStateHooksWhileBlocked(t *testing.T) { + b := newTestLocalBackend(t) + b.mu.Lock() + defer b.mu.Unlock() + b.blocked = true + var selections []tailcfg.StableNodeID + b.extHost.Hooks().ProfileStateChange.Add(func(_ ipn.LoginProfileView, prefs ipn.PrefsView, _ bool) { + selections = append(selections, prefs.ExitNodeID()) + }) + b.setPrefsLocked(&ipn.Prefs{ExitNodeID: "missing"}) + if len(selections) == 0 { + t.Fatal("prefs change did not notify extensions while blocked") + } + if got := selections[len(selections)-1]; got != "missing" { + t.Errorf("hook got exit node %q, want missing", got) + } + var peerUpdates int + b.extHost.Hooks().OnPeerUpdate.Add(func() { + peerUpdates++ + if len(b.currentNode().Peers()) != 0 { + t.Error("peer update callback did not observe the cleared peers") + } + }) + var configured []bool + b.extHost.Hooks().NetworkConfiguredChange.Add(func(v bool) { + configured = append(configured, v) + }) + b.setNetMapLocked(nil) + if peerUpdates != 1 { + t.Errorf("got %d peer update callbacks while blocked, want 1", peerUpdates) + } + if !slices.Equal(configured, []bool{false}) { + t.Errorf("network configuration events = %v, want [false]", configured) + } +} diff --git a/ipn/prefs.go b/ipn/prefs.go index 91100a3f9..a8929cbb6 100644 --- a/ipn/prefs.go +++ b/ipn/prefs.go @@ -1161,6 +1161,15 @@ func (p *LoginProfile) Equals(p2 *LoginProfile) bool { // offering the best performance will be preferred. const AnyExitNode ExitNodeExpression = "any" +// UnresolvedExitNodeID is a special [tailcfg.StableNodeID] value +// used as an exit node ID to install a blackhole route, preventing +// accidental non-exit-node usage until the [ipn.ExitNodeExpression] +// is evaluated and an actual exit node is selected. +// +// We use "auto:any" for compatibility with older, pre-[ipn.ExitNodeExpression] +// clients that have been using "auto:any" for this purpose for a long time. +const UnresolvedExitNodeID tailcfg.StableNodeID = "auto:any" + // IsSet reports whether the expression is non-empty and can be used // to select an exit node. func (e ExitNodeExpression) IsSet() bool { diff --git a/tsconst/health.go b/tsconst/health.go index dbcb8d29a..b4c3d3877 100644 --- a/tsconst/health.go +++ b/tsconst/health.go @@ -24,4 +24,5 @@ HealthWarnableApplyDiskConfig = "apply-disk-config" HealthWarnableWarmingUp = "warming-up" HealthWarnableTLSCertPending = "tls-cert-pending" + HealthWarnableExitNodeUnavailable = "exit-node-unavailable" ) diff --git a/tsnet/depaware.txt b/tsnet/depaware.txt index 3ddc57365..3a842d88e 100644 --- a/tsnet/depaware.txt +++ b/tsnet/depaware.txt @@ -148,6 +148,7 @@ tailscale.com/tsnet dependencies: (generated by github.com/tailscale/depaware) tailscale.com/feature/condregister/oauthkey from tailscale.com/tsnet tailscale.com/feature/condregister/portmapper from tailscale.com/tsnet tailscale.com/feature/condregister/useproxy from tailscale.com/tsnet + tailscale.com/feature/exitnodehealth from tailscale.com/tsnet tailscale.com/feature/favorites/pintype from tailscale.com/client/local tailscale.com/feature/netlog from tailscale.com/feature/condregister/netlog tailscale.com/feature/oauthkey from tailscale.com/feature/condregister/oauthkey diff --git a/tsnet/maybe_exitnodehealth.go b/tsnet/maybe_exitnodehealth.go new file mode 100644 index 000000000..4fe483cf1 --- /dev/null +++ b/tsnet/maybe_exitnodehealth.go @@ -0,0 +1,8 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +//go:build !ts_omit_exitnodehealth && !ts_omit_health && !ts_omit_useexitnode + +package tsnet + +import _ "tailscale.com/feature/exitnodehealth"