ipn/ipnlocal, util/winutil/policy: modify Windows profile migration to load legacy prefs from within tailscaled

I realized that a lot of the problems that we're seeing around migration and
LocalBackend state can be avoided if we drive Windows pref migration entirely
from within tailscaled. By doing it this way, tailscaled can automatically
perform the migration as soon as the connection with the client frontend is
established.

Since tailscaled is already running as LocalSystem, it already has access to
the user's local AppData directory. The profile manager already knows which
user is connected, so we simply need to resolve the user's prefs file and read
it from there.

Of course, to properly migrate this information we need to also check system
policies. I moved a bunch of policy resolution code out of the GUI and into
a new package in util/winutil/policy.

Updates #7626

Signed-off-by: Aaron Klotz <aaron@tailscale.com>

cmd/tailscaled/depaware.txt

@@ -317,6 +317,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/util/uniq                                      from tailscale.com/wgengine/magicsock+
         tailscale.com/util/vizerror                                  from tailscale.com/tsweb
      💣 tailscale.com/util/winutil                                   from tailscale.com/control/controlclient+
+   W    tailscale.com/util/winutil/policy                            from tailscale.com/ipn/ipnlocal
         tailscale.com/version                                        from tailscale.com/derp+
         tailscale.com/version/distro                                 from tailscale.com/hostinfo+
    W    tailscale.com/wf                                             from tailscale.com/cmd/tailscaled

ipn/ipnlocal/profiles.go

@@ -20,7 +20,6 @@ import (
 	"tailscale.com/types/logger"
 	"tailscale.com/util/clientmetric"
 	"tailscale.com/util/winutil"
-	"tailscale.com/version"
 )
 
 // profileManager is a wrapper around a StateStore that manages
@@ -66,7 +65,13 @@ func (pm *profileManager) SetCurrentUserID(uid ipn.WindowsUserID) error {
 	// the selected profile for the current user.
 	b, err := pm.store.ReadState(ipn.CurrentProfileKey(string(uid)))
 	if err == ipn.ErrStateNotExist || len(b) == 0 {
-		pm.NewProfile()
+		if runtime.GOOS == "windows" {
+			if err := pm.migrateFromLegacyPrefs(); err != nil {
+				return err
+			}
+		} else {
+			pm.NewProfile()
+		}
 		return nil
 	}
 
@@ -424,12 +429,7 @@ var defaultPrefs = func() ipn.PrefsView {
 	prefs.WantRunning = false
 
 	prefs.ControlURL = winutil.GetPolicyString("LoginURL", "")
-
+	prefs.ExitNodeIP = resolveExitNodeIP(netip.Addr{})
-	if exitNode := winutil.GetPolicyString("ExitNodeIP", ""); exitNode != "" {
-		if ip, err := netip.ParseAddr(exitNode); err == nil {
-			prefs.ExitNodeIP = ip
-		}
-	}
 
 	// Allow Incoming (used by the UI) is the negation of ShieldsUp (used by the
 	// backend), so this has to convert between the two conventions.
@@ -439,6 +439,16 @@ var defaultPrefs = func() ipn.PrefsView {
 	return prefs.View()
 }()
 
+func resolveExitNodeIP(defIP netip.Addr) (ret netip.Addr) {
+	ret = defIP
+	if exitNode := winutil.GetPolicyString("ExitNodeIP", ""); exitNode != "" {
+		if ip, err := netip.ParseAddr(exitNode); err == nil {
+			ret = ip
+		}
+	}
+	return ret
+}
+
 // Store returns the StateStore used by the ProfileManager.
 func (pm *profileManager) Store() ipn.StateStore {
 	return pm.store
@@ -549,27 +559,16 @@ func newProfileManagerWithGOOS(store ipn.StateStore, logf logger.Logf, goos stri
 func (pm *profileManager) migrateFromLegacyPrefs() error {
 	metricMigration.Add(1)
 	pm.NewProfile()
-	k := ipn.LegacyGlobalDaemonStateKey
+	sentinel, prefs, err := pm.loadLegacyPrefs()
-	switch {
-	case runtime.GOOS == "ios":
-		k = "ipn-go-bridge"
-	case version.IsSandboxedMacOS():
-		k = "ipn-go-bridge"
-	case runtime.GOOS == "android":
-		k = "ipn-android"
-	}
-	prefs, err := pm.loadSavedPrefs(k)
 	if err != nil {
 		metricMigrationError.Add(1)
-		return fmt.Errorf("calling ReadState on state store: %w", err)
+		return err
 	}
-	pm.logf("migrating %q profile to new format", k)
 	if err := pm.SetPrefs(prefs); err != nil {
 		metricMigrationError.Add(1)
 		return fmt.Errorf("migrating _daemon profile: %w", err)
 	}
-	// Do not delete the old state key, as we may be downgraded to an
+	pm.completeMigration(sentinel)
-	// older version that still relies on it.
 	metricMigrationSuccess.Add(1)
 	return nil
 }

ipn/ipnlocal/profiles_notwindows.go

@@ -0,0 +1,37 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+//go:build !windows
+
+package ipnlocal
+
+import (
+	"fmt"
+	"runtime"
+
+	"tailscale.com/ipn"
+	"tailscale.com/version"
+)
+
+func (pm *profileManager) loadLegacyPrefs() (string, ipn.PrefsView, error) {
+	k := ipn.LegacyGlobalDaemonStateKey
+	switch {
+	case runtime.GOOS == "ios":
+		k = "ipn-go-bridge"
+	case version.IsSandboxedMacOS():
+		k = "ipn-go-bridge"
+	case runtime.GOOS == "android":
+		k = "ipn-android"
+	}
+	prefs, err := pm.loadSavedPrefs(k)
+	if err != nil {
+		return "", ipn.PrefsView{}, fmt.Errorf("calling ReadState on state store: %w", err)
+	}
+	pm.logf("migrating %q profile to new format", k)
+	return "", prefs, nil
+}
+
+func (pm *profileManager) completeMigration(migrationSentinel string) {
+	// Do not delete the old state key, as we may be downgraded to an
+	// older version that still relies on it.
+}

ipn/ipnlocal/profiles_windows.go

@@ -0,0 +1,84 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+package ipnlocal
+
+import (
+	"errors"
+	"fmt"
+	"os"
+	"os/user"
+	"path/filepath"
+
+	"tailscale.com/atomicfile"
+	"tailscale.com/ipn"
+	"tailscale.com/util/winutil/policy"
+)
+
+const (
+	legacyPrefsFile                  = "prefs"
+	legacyPrefsMigrationSentinelFile = "_migrated-to-profiles"
+	legacyPrefsExt                   = ".conf"
+)
+
+var errAlreadyMigrated = errors.New("profile migration already completed")
+
+func legacyPrefsDir(uid ipn.WindowsUserID) (string, error) {
+	// TODO(aaron): Ideally we'd have the impersonation token for the pipe's
+	// client and use it to call SHGetKnownFolderPath, thus yielding the correct
+	// path without having to make gross assumptions about directory names.
+	usr, err := user.LookupId(string(uid))
+	if err != nil {
+		return "", err
+	}
+	if usr.HomeDir == "" {
+		return "", fmt.Errorf("user %q does not have a home directory", uid)
+	}
+	userLegacyPrefsDir := filepath.Join(usr.HomeDir, "AppData", "Local", "Tailscale")
+	return userLegacyPrefsDir, nil
+}
+
+func (pm *profileManager) loadLegacyPrefs() (string, ipn.PrefsView, error) {
+	userLegacyPrefsDir, err := legacyPrefsDir(pm.currentUserID)
+	if err != nil {
+		return "", ipn.PrefsView{}, err
+	}
+
+	migrationSentinel := filepath.Join(userLegacyPrefsDir, legacyPrefsMigrationSentinelFile+legacyPrefsExt)
+	// verify that migration sentinel is not present
+	_, err = os.Stat(migrationSentinel)
+	if err == nil {
+		return "", ipn.PrefsView{}, errAlreadyMigrated
+	}
+	if !os.IsNotExist(err) {
+		return "", ipn.PrefsView{}, err
+	}
+
+	prefsPath := filepath.Join(userLegacyPrefsDir, legacyPrefsFile+legacyPrefsExt)
+	prefs, err := ipn.LoadPrefs(prefsPath)
+	if err != nil {
+		return "", ipn.PrefsView{}, err
+	}
+
+	prefs.ControlURL = policy.SelectControlURL(defaultPrefs.ControlURL(), prefs.ControlURL)
+	prefs.ExitNodeIP = resolveExitNodeIP(prefs.ExitNodeIP)
+	prefs.ShieldsUp = resolveShieldsUp(prefs.ShieldsUp)
+	prefs.ForceDaemon = resolveForceDaemon(prefs.ForceDaemon)
+
+	pm.logf("migrating Windows profile to new format")
+	return migrationSentinel, prefs.View(), nil
+}
+
+func (pm *profileManager) completeMigration(migrationSentinel string) {
+	atomicfile.WriteFile(migrationSentinel, []byte{}, 0600)
+}
+
+func resolveShieldsUp(defval bool) bool {
+	pol := policy.GetPreferenceOptionPolicy("AllowIncomingConnections")
+	return !pol.ShouldEnable(!defval)
+}
+
+func resolveForceDaemon(defval bool) bool {
+	pol := policy.GetPreferenceOptionPolicy("UnattendedMode")
+	return pol.ShouldEnable(defval)
+}

util/winutil/policy/policy_windows.go

@@ -0,0 +1,150 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+// Package policy contains higher-level abstractions for accessing Windows enterprise policies.
+package policy
+
+import (
+	"time"
+
+	"tailscale.com/util/winutil"
+)
+
+// PreferenceOptionPolicy is a policy that governs whether a boolean variable
+// is forcibly assigned an administrator-defined value, or allowed to receive
+// a user-defined value.
+type PreferenceOptionPolicy int
+
+const (
+	showChoiceByPolicy PreferenceOptionPolicy = iota
+	neverByPolicy
+	alwaysByPolicy
+)
+
+// Show returns if the UI option that controls the choice administered by this
+// policy should be shown. Currently this is true if and only if the policy is
+// showChoiceByPolicy.
+func (p PreferenceOptionPolicy) Show() bool {
+	return p == showChoiceByPolicy
+}
+
+// ShouldEnable checks if the choice administered by this policy should be
+// enabled. If the administrator has chosen a setting, the administrator's
+// setting is returned, otherwise userChoice is returned.
+func (p PreferenceOptionPolicy) ShouldEnable(userChoice bool) bool {
+	switch p {
+	case neverByPolicy:
+		return false
+	case alwaysByPolicy:
+		return true
+	default:
+		return userChoice
+	}
+}
+
+// GetPreferenceOptionPolicy loads a policy from the registry that can be
+// managed by an enterprise policy management system and allows administrative
+// overrides of users' choices in a way that we do not want tailcontrol to have
+// the authority to set. It describes user-decides/always/never options, where
+// "always" and "never" remove the user's ability to make a selection. If not
+// present or set to a different value, "user-decides" is the default.
+func GetPreferenceOptionPolicy(name string) PreferenceOptionPolicy {
+	opt := winutil.GetPolicyString(name, "user-decides")
+	switch opt {
+	case "always":
+		return alwaysByPolicy
+	case "never":
+		return neverByPolicy
+	default:
+		return showChoiceByPolicy
+	}
+}
+
+// VisibilityPolicy is a policy that controls whether or not a particular
+// component of a user interface is to be shown.
+type VisibilityPolicy byte
+
+const (
+	visibleByPolicy VisibilityPolicy = 'v'
+	hiddenByPolicy  VisibilityPolicy = 'h'
+)
+
+// Show reports whether the UI option administered by this policy should be shown.
+// Currently this is true if and only if the policy is visibleByPolicy.
+func (p VisibilityPolicy) Show() bool {
+	return p == visibleByPolicy
+}
+
+// GetVisibilityPolicy loads a policy from the registry that can be managed
+// by an enterprise policy management system and describes show/hide decisions
+// for UI elements. The registry value should be a string set to "show" (return
+// true) or "hide" (return true). If not present or set to a different value,
+// "show" (return false) is the default.
+func GetVisibilityPolicy(name string) VisibilityPolicy {
+	opt := winutil.GetPolicyString(name, "show")
+	switch opt {
+	case "hide":
+		return hiddenByPolicy
+	default:
+		return visibleByPolicy
+	}
+}
+
+// GetDurationPolicy loads a policy from the registry that can be managed
+// by an enterprise policy management system and describes a duration for some
+// action. The registry value should be a string that time.ParseDuration
+// understands. If the registry value is "" or can not be processed,
+// defaultValue is returned instead.
+func GetDurationPolicy(name string, defaultValue time.Duration) time.Duration {
+	opt := winutil.GetPolicyString(name, "")
+	if opt == "" {
+		return defaultValue
+	}
+	v, err := time.ParseDuration(opt)
+	if err != nil || v < 0 {
+		return defaultValue
+	}
+	return v
+}
+
+// SelectControlURL returns the ControlURL to use based on a value in
+// the registry (LoginURL) and the one on disk (in the GUI's
+// prefs.conf). If both are empty, it returns a default value. (It
+// always return a non-empty value)
+//
+// See https://github.com/tailscale/tailscale/issues/2798 for some background.
+func SelectControlURL(reg, disk string) string {
+	const def = "https://controlplane.tailscale.com"
+
+	// Prior to Dec 2020's commit 739b02e6, the installer
+	// wrote a LoginURL value of https://login.tailscale.com to the registry.
+	const oldRegDef = "https://login.tailscale.com"
+
+	// If they have an explicit value in the registry, use it,
+	// unless it's an old default value from an old installer.
+	// Then we have to see which is better.
+	if reg != "" {
+		if reg != oldRegDef {
+			// Something explicit in the registry that we didn't
+			// set ourselves by the installer.
+			return reg
+		}
+		if disk == "" {
+			// Something in the registry is better than nothing on disk.
+			return reg
+		}
+		if disk != def && disk != oldRegDef {
+			// The value in the registry is the old
+			// default (login.tailscale.com) but the value
+			// on disk is neither our old nor new default
+			// value, so it must be some custom thing that
+			// the user cares about. Prefer the disk value.
+			return disk
+		}
+	}
+	if disk != "" {
+		return disk
+	}
+	return def
+}
+

util/winutil/policy/policy_windows_test.go

@@ -0,0 +1,38 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+package policy
+
+import "testing"
+
+func TestSelectControlURL(t *testing.T) {
+	tests := []struct {
+		reg, disk, want string
+	}{
+		// Modern default case.
+		{"", "", "https://controlplane.tailscale.com"},
+
+		// For a user who installed prior to Dec 2020, with
+		// stuff in their registry.
+		{"https://login.tailscale.com", "", "https://login.tailscale.com"},
+
+		// Ignore pre-Dec'20 LoginURL from installer if prefs
+		// prefs overridden manually to an on-prem control
+		// server.
+		{"https://login.tailscale.com", "http://on-prem", "http://on-prem"},
+
+		// Something unknown explicitly set in the registry always wins.
+		{"http://explicit-reg", "", "http://explicit-reg"},
+		{"http://explicit-reg", "http://on-prem", "http://explicit-reg"},
+		{"http://explicit-reg", "https://login.tailscale.com", "http://explicit-reg"},
+		{"http://explicit-reg", "https://controlplane.tailscale.com", "http://explicit-reg"},
+
+		// If nothing in the registry, disk wins.
+		{"", "http://on-prem", "http://on-prem"},
+	}
+	for _, tt := range tests {
+		if got := SelectControlURL(tt.reg, tt.disk); got != tt.want {
+			t.Errorf("(reg %q, disk %q) = %q; want %q", tt.reg, tt.disk, got, tt.want)
+		}
+	}
+}