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"