From a5e1f7d70364b3cb7dbfd1d670d42f9d447435d5 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 3 Apr 2024 10:51:51 -0700 Subject: [PATCH] ipn/{ipnlocal,localapi}: add API to toggle use of exit node This is primarily for GUIs, so they don't need to remember the most recently used exit node themselves. This adds some CLI commands, but they're disabled and behind the WIP envknob, as we need to consider naming (on/off is ambiguous with running an exit node, etc) as well as automatic exit node selection in the future. For now the CLI commands are effectively developer debug things to test the LocalAPI. Updates tailscale/corp#18724 Change-Id: I9a32b00e3ffbf5b29bfdcad996a4296b5e37be7e Signed-off-by: Brad Fitzpatrick --- client/tailscale/localclient.go | 9 +++++ cmd/tailscale/cli/cli_test.go | 4 +++ cmd/tailscale/cli/exitnode.go | 50 ++++++++++++++++++++++++---- ipn/ipn_clone.go | 1 + ipn/ipn_view.go | 2 ++ ipn/ipnlocal/local.go | 58 ++++++++++++++++++++++++++++++--- ipn/ipnlocal/local_test.go | 55 +++++++++++++++++++++++++++++++ ipn/localapi/localapi.go | 27 +++++++++++++++ ipn/prefs.go | 16 +++++++++ ipn/prefs_test.go | 14 ++++++++ 10 files changed, 225 insertions(+), 11 deletions(-) diff --git a/client/tailscale/localclient.go b/client/tailscale/localclient.go index 2bc4fa16d..bb3b0c216 100644 --- a/client/tailscale/localclient.go +++ b/client/tailscale/localclient.go @@ -1418,6 +1418,15 @@ func (lc *LocalClient) CheckUpdate(ctx context.Context) (*tailcfg.ClientVersion, return &cv, nil } +// SetUseExitNode toggles the use of an exit node on or off. +// To turn it on, there must have been a previously used exit node. +// The most previously used one is reused. +// This is a convenience method for GUIs. To select an actual one, update the prefs. +func (lc *LocalClient) SetUseExitNode(ctx context.Context, on bool) error { + _, err := lc.send(ctx, "POST", "/localapi/v0/set-use-exit-node-enabled?enabled="+strconv.FormatBool(on), http.StatusOK, nil) + return err +} + // DriveSetServerAddr instructs Taildrive to use the server at addr to access // the filesystem. This is used on platforms like Windows and MacOS to let // Taildrive know to use the file server running in the GUI app. diff --git a/cmd/tailscale/cli/cli_test.go b/cmd/tailscale/cli/cli_test.go index 7c1534b39..d06a6ea3d 100644 --- a/cmd/tailscale/cli/cli_test.go +++ b/cmd/tailscale/cli/cli_test.go @@ -833,6 +833,10 @@ func TestPrefFlagMapping(t *testing.T) { // Handled by the tailscale share subcommand, we don't want a CLI // flag for this. continue + case "InternalExitNodePrior": + // Used internally by LocalBackend as part of exit node usage toggling. + // No CLI flag for this. + continue } t.Errorf("unexpected new ipn.Pref field %q is not handled by up.go (see addPrefFlagMapping and checkForAccidentalSettingReverts)", prefName) } diff --git a/cmd/tailscale/cli/exitnode.go b/cmd/tailscale/cli/exitnode.go index 587ff837c..370c9207e 100644 --- a/cmd/tailscale/cli/exitnode.go +++ b/cmd/tailscale/cli/exitnode.go @@ -16,6 +16,7 @@ import ( "github.com/peterbourgon/ff/v3/ffcli" xmaps "golang.org/x/exp/maps" + "tailscale.com/envknob" "tailscale.com/ipn/ipnstate" "tailscale.com/tailcfg" ) @@ -25,7 +26,10 @@ var exitNodeCmd = &ffcli.Command{ ShortUsage: "exit-node [flags]", ShortHelp: "Show machines on your tailnet configured as exit nodes", LongHelp: "Show machines on your tailnet configured as exit nodes", - Subcommands: []*ffcli.Command{ + Exec: func(context.Context, []string) error { + return errors.New("exit-node subcommand required; run 'tailscale exit-node -h' for details") + }, + Subcommands: append([]*ffcli.Command{ { Name: "list", ShortUsage: "exit-node list [flags]", @@ -36,17 +40,51 @@ var exitNodeCmd = &ffcli.Command{ fs.StringVar(&exitNodeArgs.filter, "filter", "", "filter exit nodes by country") return fs })(), - }, - }, - Exec: func(context.Context, []string) error { - return errors.New("exit-node subcommand required; run 'tailscale exit-node -h' for details") - }, + }}, + (func() []*ffcli.Command { + if !envknob.UseWIPCode() { + return nil + } + return []*ffcli.Command{ + { + Name: "connect", + ShortUsage: "exit-node connect", + ShortHelp: "connect to most recently used exit node", + Exec: exitNodeSetUse(true), + }, + { + Name: "disconnect", + ShortUsage: "exit-node disconnect", + ShortHelp: "disconnect from current exit node, if any", + Exec: exitNodeSetUse(false), + }, + } + })()...), } var exitNodeArgs struct { filter string } +func exitNodeSetUse(wantOn bool) func(ctx context.Context, args []string) error { + return func(ctx context.Context, args []string) error { + if len(args) > 0 { + return errors.New("unexpected non-flag arguments") + } + err := localClient.SetUseExitNode(ctx, wantOn) + if err != nil { + if !wantOn { + pref, err := localClient.GetPrefs(ctx) + if err == nil && pref.ExitNodeID == "" { + // Two processes concurrently turned it off. + return nil + } + } + } + return err + } +} + // runExitNodeList returns a formatted list of exit nodes for a tailnet. // If the exit node has location and priority data, only the highest // priority node for each city location is shown to the user. diff --git a/ipn/ipn_clone.go b/ipn/ipn_clone.go index 0abeaa700..cba440da0 100644 --- a/ipn/ipn_clone.go +++ b/ipn/ipn_clone.go @@ -42,6 +42,7 @@ var _PrefsCloneNeedsRegeneration = Prefs(struct { AllowSingleHosts bool ExitNodeID tailcfg.StableNodeID ExitNodeIP netip.Addr + InternalExitNodePrior string ExitNodeAllowLANAccess bool CorpDNS bool RunSSH bool diff --git a/ipn/ipn_view.go b/ipn/ipn_view.go index 75370dae6..6a121ad83 100644 --- a/ipn/ipn_view.go +++ b/ipn/ipn_view.go @@ -69,6 +69,7 @@ func (v PrefsView) RouteAll() bool { return v.ж.RouteAll } func (v PrefsView) AllowSingleHosts() bool { return v.ж.AllowSingleHosts } func (v PrefsView) ExitNodeID() tailcfg.StableNodeID { return v.ж.ExitNodeID } func (v PrefsView) ExitNodeIP() netip.Addr { return v.ж.ExitNodeIP } +func (v PrefsView) InternalExitNodePrior() string { return v.ж.InternalExitNodePrior } func (v PrefsView) ExitNodeAllowLANAccess() bool { return v.ж.ExitNodeAllowLANAccess } func (v PrefsView) CorpDNS() bool { return v.ж.CorpDNS } func (v PrefsView) RunSSH() bool { return v.ж.RunSSH } @@ -104,6 +105,7 @@ var _PrefsViewNeedsRegeneration = Prefs(struct { AllowSingleHosts bool ExitNodeID tailcfg.StableNodeID ExitNodeIP netip.Addr + InternalExitNodePrior string ExitNodeAllowLANAccess bool CorpDNS bool RunSSH bool diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index bb907ca77..f5277ff8d 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -3158,9 +3158,59 @@ func (b *LocalBackend) checkFunnelEnabledLocked(p *ipn.Prefs) error { return nil } +// SetUseExitNodeEnabled turns on or off the most recently selected exit node. +// +// On success, it returns the resulting prefs (or current prefs, in the case of no change). +// Setting the value to false when use of an exit node is already false is not an error, +// nor is true when the exit node is already in use. +func (b *LocalBackend) SetUseExitNodeEnabled(v bool) (ipn.PrefsView, error) { + unlock := b.lockAndGetUnlock() + defer unlock() + + p0 := b.pm.CurrentPrefs() + if v && p0.ExitNodeID() != "" { + // Already on. + return p0, nil + } + if !v && p0.ExitNodeID() == "" { + // Already off. + return p0, nil + } + + var zero ipn.PrefsView + if v && p0.InternalExitNodePrior() == "" { + if !p0.ExitNodeIP().IsValid() { + return zero, errors.New("no exit node IP to enable & prior exit node IP was never resolved an a node") + } + return zero, errors.New("no prior exit node to enable") + } + + mp := &ipn.MaskedPrefs{} + if v { + mp.ExitNodeIDSet = true + mp.ExitNodeID = tailcfg.StableNodeID(p0.InternalExitNodePrior()) + } else { + mp.ExitNodeIDSet = true + mp.ExitNodeID = "" + mp.InternalExitNodePriorSet = true + mp.InternalExitNodePrior = string(p0.ExitNodeID()) + } + return b.editPrefsLockedOnEntry(mp, unlock) +} + func (b *LocalBackend) EditPrefs(mp *ipn.MaskedPrefs) (ipn.PrefsView, error) { + if mp.SetsInternal() { + return ipn.PrefsView{}, errors.New("can't set Internal fields") + } unlock := b.lockAndGetUnlock() defer unlock() + return b.editPrefsLockedOnEntry(mp, unlock) +} + +// Warning: b.mu must be held on entry, but it unlocks it on the way out. +// TODO(bradfitz): redo the locking on all these weird methods like this. +func (b *LocalBackend) editPrefsLockedOnEntry(mp *ipn.MaskedPrefs, unlock unlockOnce) (ipn.PrefsView, error) { + defer unlock() // for error paths if mp.EggSet { mp.EggSet = false @@ -4651,18 +4701,16 @@ func (b *LocalBackend) Logout(ctx context.Context) error { // Grab the current profile before we unlock the mutex, so that we can // delete it later. profile := b.pm.CurrentProfile() - unlock.UnlockEarly() - // TODO(bradfitz): call/make editPrefsLocked here and stay locked until - // before the cc.Logout. - _, err := b.EditPrefs(&ipn.MaskedPrefs{ + _, err := b.editPrefsLockedOnEntry(&ipn.MaskedPrefs{ WantRunningSet: true, LoggedOutSet: true, Prefs: ipn.Prefs{WantRunning: false, LoggedOut: true}, - }) + }, unlock) if err != nil { return err } + // b.mu is now unlocked, after editPrefsLockedOnEntry. // Clear any previous dial plan(s), if set. b.dialPlan.Store(nil) diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index aad62bad9..e877dbd83 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -455,6 +455,61 @@ func TestLazyMachineKeyGeneration(t *testing.T) { time.Sleep(500 * time.Millisecond) } +func TestSetUseExitNodeEnabled(t *testing.T) { + lb := newTestLocalBackend(t) + + // Can't turn it on if it never had an old value. + if _, err := lb.SetUseExitNodeEnabled(true); err == nil { + t.Fatal("expected success") + } + + // But we can turn it off when it's already off. + if _, err := lb.SetUseExitNodeEnabled(false); err != nil { + t.Fatal("expected failure") + } + + // Give it an initial exit node in use. + if _, err := lb.EditPrefs(&ipn.MaskedPrefs{ + ExitNodeIDSet: true, + Prefs: ipn.Prefs{ + ExitNodeID: "foo", + }, + }); err != nil { + t.Fatalf("enabling first exit node: %v", err) + } + + // Now turn off that exit node. + if prefs, err := lb.SetUseExitNodeEnabled(false); err != nil { + t.Fatal("expected failure") + } else { + if g, w := prefs.ExitNodeID(), tailcfg.StableNodeID(""); g != w { + t.Fatalf("unexpected exit node ID %q; want %q", g, w) + } + if g, w := prefs.InternalExitNodePrior(), "foo"; g != w { + t.Fatalf("unexpected exit node prior %q; want %q", g, w) + } + } + + // And turn it back on. + if prefs, err := lb.SetUseExitNodeEnabled(true); err != nil { + t.Fatal("expected failure") + } else { + if g, w := prefs.ExitNodeID(), tailcfg.StableNodeID("foo"); g != w { + t.Fatalf("unexpected exit node ID %q; want %q", g, w) + } + if g, w := prefs.InternalExitNodePrior(), "foo"; g != w { + t.Fatalf("unexpected exit node prior %q; want %q", g, w) + } + } + + // Verify we block setting an Internal field. + if _, err := lb.EditPrefs(&ipn.MaskedPrefs{ + InternalExitNodePriorSet: true, + }); err == nil { + t.Fatalf("unexpected success; want an error trying to set an internal field") + } +} + func TestFileTargets(t *testing.T) { b := new(LocalBackend) _, err := b.FileTargets() diff --git a/ipn/localapi/localapi.go b/ipn/localapi/localapi.go index 32c65f9d5..6910bf1d9 100644 --- a/ipn/localapi/localapi.go +++ b/ipn/localapi/localapi.go @@ -119,6 +119,7 @@ var handler = map[string]localAPIHandler{ "set-expiry-sooner": (*Handler).serveSetExpirySooner, "set-gui-visible": (*Handler).serveSetGUIVisible, "set-push-device-token": (*Handler).serveSetPushDeviceToken, + "set-use-exit-node-enabled": (*Handler).serveSetUseExitNodeEnabled, "start": (*Handler).serveStart, "status": (*Handler).serveStatus, "tka/affected-sigs": (*Handler).serveTKAAffectedSigs, @@ -2108,6 +2109,32 @@ func (h *Handler) serveSetGUIVisible(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) } +func (h *Handler) serveSetUseExitNodeEnabled(w http.ResponseWriter, r *http.Request) { + if r.Method != httpm.POST { + http.Error(w, "use POST", http.StatusMethodNotAllowed) + return + } + if !h.PermitWrite { + http.Error(w, "access denied", http.StatusForbidden) + return + } + + v, err := strconv.ParseBool(r.URL.Query().Get("enabled")) + if err != nil { + http.Error(w, "invalid 'enabled' parameter", http.StatusBadRequest) + return + } + prefs, err := h.b.SetUseExitNodeEnabled(v) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + w.Header().Set("Content-Type", "application/json") + e := json.NewEncoder(w) + e.SetIndent("", "\t") + e.Encode(prefs) +} + func (h *Handler) serveTKASign(w http.ResponseWriter, r *http.Request) { if !h.PermitWrite { http.Error(w, "lock sign access denied", http.StatusForbidden) diff --git a/ipn/prefs.go b/ipn/prefs.go index ef81cd08b..2fa605a50 100644 --- a/ipn/prefs.go +++ b/ipn/prefs.go @@ -105,6 +105,14 @@ type Prefs struct { ExitNodeID tailcfg.StableNodeID ExitNodeIP netip.Addr + // InternalExitNodePrior is the most recently used ExitNodeID in string form. It is set by + // the backend on transition from exit node on to off and used by the + // backend. It's not of type tailcfg.StableNodeID because in the future we plan + // to overload this field to mean things like "Anything in country $FOO" too. + // + // As an Internal field, it can't be set by LocalAPI clients. + InternalExitNodePrior string + // ExitNodeAllowLANAccess indicates whether locally accessible subnets should be // routed directly or via the exit node. ExitNodeAllowLANAccess bool @@ -279,6 +287,7 @@ type MaskedPrefs struct { AllowSingleHostsSet bool `json:",omitempty"` ExitNodeIDSet bool `json:",omitempty"` ExitNodeIPSet bool `json:",omitempty"` + InternalExitNodePriorSet bool `json:",omitempty"` // Internal; can't be set by LocalAPI clients ExitNodeAllowLANAccessSet bool `json:",omitempty"` CorpDNSSet bool `json:",omitempty"` RunSSHSet bool `json:",omitempty"` @@ -303,6 +312,12 @@ type MaskedPrefs struct { DriveSharesSet bool `json:",omitempty"` } +// SetsInternal reports whether mp has any of the Internal*Set field bools set +// to true. +func (mp *MaskedPrefs) SetsInternal() bool { + return mp.InternalExitNodePriorSet +} + type AutoUpdatePrefsMask struct { CheckSet bool `json:",omitempty"` ApplySet bool `json:",omitempty"` @@ -544,6 +559,7 @@ func (p *Prefs) Equals(p2 *Prefs) bool { p.AllowSingleHosts == p2.AllowSingleHosts && p.ExitNodeID == p2.ExitNodeID && p.ExitNodeIP == p2.ExitNodeIP && + p.InternalExitNodePrior == p2.InternalExitNodePrior && p.ExitNodeAllowLANAccess == p2.ExitNodeAllowLANAccess && p.CorpDNS == p2.CorpDNS && p.RunSSH == p2.RunSSH && diff --git a/ipn/prefs_test.go b/ipn/prefs_test.go index 1d283a57f..29c3f2479 100644 --- a/ipn/prefs_test.go +++ b/ipn/prefs_test.go @@ -41,6 +41,7 @@ func TestPrefsEqual(t *testing.T) { "AllowSingleHosts", "ExitNodeID", "ExitNodeIP", + "InternalExitNodePrior", "ExitNodeAllowLANAccess", "CorpDNS", "RunSSH", @@ -614,6 +615,19 @@ func TestLoadPrefsFileWithZeroInIt(t *testing.T) { t.Fatalf("unexpected prefs=%#v, err=%v", p, err) } +func TestMaskedPrefsSetsInternal(t *testing.T) { + for _, f := range fieldsOf(reflect.TypeFor[MaskedPrefs]()) { + if !strings.HasSuffix(f, "Set") || !strings.HasPrefix(f, "Internal") { + continue + } + mp := new(MaskedPrefs) + reflect.ValueOf(mp).Elem().FieldByName(f).SetBool(true) + if !mp.SetsInternal() { + t.Errorf("MaskedPrefs.%sSet=true but SetsInternal=false", f) + } + } +} + func TestMaskedPrefsFields(t *testing.T) { have := map[string]bool{} for _, f := range fieldsOf(reflect.TypeFor[Prefs]()) {