logpolicy: don't use C:\ProgramData use for tailscale-ipn GUI's log dir

tailscale-ipn.exe (the GUI) shouldn't use C:\ProgramData.

Also, migrate the earlier misnamed wg32/wg64 conf files if they're present.
(That was stopped in 2db877caa3, but the
files exist from fresh 1.14 installs)

Updates #2856

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

ipn/ipnserver/server.go

@@ -595,14 +595,14 @@ func (s *server) writeToClients(n ipn.Notify) {
 // Returns a string of the path to use for the state file.
 // This will be a fallback %LocalAppData% path if migration fails,
 // a %ProgramData% path otherwise.
-func tryWindowsAppDataMigration(path string) string {
+func tryWindowsAppDataMigration(logf logger.Logf, path string) string {
 	if path != paths.DefaultTailscaledStateFile() {
 		// If they're specifying a non-default path, just trust that they know
 		// what they are doing.
 		return path
 	}
 	oldFile := filepath.Join(os.Getenv("LocalAppData"), "Tailscale", "server-state.conf")
-	return paths.TryConfigFileMigration(oldFile, path)
+	return paths.TryConfigFileMigration(logf, oldFile, path)
 }
 
 // Run runs a Tailscale backend service.
@@ -648,7 +648,7 @@ func Run(ctx context.Context, logf logger.Logf, logid string, getEngine func() (
 			}
 		default:
 			if runtime.GOOS == "windows" {
-				path = tryWindowsAppDataMigration(path)
+				path = tryWindowsAppDataMigration(logf, path)
 			}
 			store, err = ipn.NewFileStore(path)
 			if err != nil {

logpolicy/logpolicy.go

@@ -135,12 +135,33 @@ func logsDir(logf logger.Logf) string {
 		}
 	}
 
-	// STATE_DIRECTORY is set by systemd 240+ but we support older
+	switch runtime.GOOS {
-	// systems-d. For example, Ubuntu 18.04 (Bionic Beaver) is 237.
+	case "windows":
-	systemdStateDir := os.Getenv("STATE_DIRECTORY")
+		if version.CmdName() == "tailscaled" {
-	if systemdStateDir != "" {
+			// In the common case, when tailscaled is run as the Local System (as a service),
-		logf("logpolicy: using $STATE_DIRECTORY, %q", systemdStateDir)
+			// we want to use %ProgramData% (C:\ProgramData\Tailscale), aside the
-		return systemdStateDir
+			// system state config with the machine key, etc. But if that directory's
+			// not accessible, then it's probably because the user is running tailscaled
+			// as a regular user (perhaps in userspace-networking/SOCK5 mode) and we should
+			// just use the %LocalAppData% instead. In a user context, %LocalAppData% isn't
+			// subject to random deletions from Windows system updates.
+			dir := filepath.Join(os.Getenv("ProgramData"), "Tailscale")
+			if winProgramDataAccessible(dir) {
+				logf("logpolicy: using dir %v", dir)
+				return dir
+			}
+		}
+		dir := filepath.Join(os.Getenv("LocalAppData"), "Tailscale")
+		logf("logpolicy: using LocalAppData dir %v", dir)
+		return dir
+	case "linux":
+		// STATE_DIRECTORY is set by systemd 240+ but we support older
+		// systems-d. For example, Ubuntu 18.04 (Bionic Beaver) is 237.
+		systemdStateDir := os.Getenv("STATE_DIRECTORY")
+		if systemdStateDir != "" {
+			logf("logpolicy: using $STATE_DIRECTORY, %q", systemdStateDir)
+			return systemdStateDir
+		}
 	}
 
 	// Default to e.g. /var/lib/tailscale or /var/db/tailscale on Unix.
@@ -191,6 +212,23 @@ func redirectStderrToLogPanics() bool {
 	return runningUnderSystemd() || os.Getenv("TS_PLEASE_PANIC") != ""
 }
 
+// winProgramDataAccessible reports whether the directory (assumed to
+// be a Windows %ProgramData% directory) is accessible to the current
+// process. It's created if needed.
+func winProgramDataAccessible(dir string) bool {
+	if err := os.MkdirAll(dir, 0700); err != nil {
+		// TODO: windows ACLs
+		return false
+	}
+	// The C:\ProgramData\Tailscale directory should be locked down
+	// by with ACLs to only be readable by the local system so a
+	// regular user shouldn't be able to do this operation:
+	if _, err := os.ReadDir(dir); err != nil {
+		return false
+	}
+	return true
+}
+
 // tryFixLogStateLocation is a temporary fixup for
 // https://github.com/tailscale/tailscale/issues/247 . We accidentally
 // wrote logging state files to /, and then later to $CACHE_DIRECTORY
@@ -372,34 +410,45 @@ func New(collection string) *Policy {
 
 	cfgPath := filepath.Join(dir, fmt.Sprintf("%s.log.conf", cmdName))
 
-	if runtime.GOOS == "windows" && cmdName == "tailscaled" {
+	if runtime.GOOS == "windows" {
-		// Tailscale 1.14 and before stored state under %LocalAppData%
+		switch cmdName {
-		// (usually "C:\WINDOWS\system32\config\systemprofile\AppData\Local"
+		case "tailscaled":
-		// when tailscaled.exe is running as a non-user system service).
+			// Tailscale 1.14 and before stored state under %LocalAppData%
-		// However it is frequently cleared for almost any reason: Windows
+			// (usually "C:\WINDOWS\system32\config\systemprofile\AppData\Local"
-		// updates, System Restore, even various System Cleaner utilities.
+			// when tailscaled.exe is running as a non-user system service).
-		//
+			// However it is frequently cleared for almost any reason: Windows
-		// The Windows service previously ran as tailscale-ipn.exe, so
+			// updates, System Restore, even various System Cleaner utilities.
-		// machines which ran very old versions might still have their
+			//
-		// log conf named %LocalAppData%\tailscale-ipn.log.conf
+			// The Windows service previously ran as tailscale-ipn.exe, so
-		//
+			// machines which ran very old versions might still have their
-		// Machines which started using Tailscale more recently will have
+			// log conf named %LocalAppData%\tailscale-ipn.log.conf
-		// %LocalAppData%\tailscaled.log.conf
+			//
-		//
+			// Machines which started using Tailscale more recently will have
-		// Attempt to migrate the log conf to C:\ProgramData\Tailscale
+			// %LocalAppData%\tailscaled.log.conf
-		oldDir := filepath.Join(os.Getenv("LocalAppData"), "Tailscale")
+			//
+			// Attempt to migrate the log conf to C:\ProgramData\Tailscale
+			oldDir := filepath.Join(os.Getenv("LocalAppData"), "Tailscale")
 
-		oldPath := filepath.Join(oldDir, "tailscaled.log.conf")
+			oldPath := filepath.Join(oldDir, "tailscaled.log.conf")
-		if fi, err := os.Stat(oldPath); err != nil || !fi.Mode().IsRegular() {
+			if fi, err := os.Stat(oldPath); err != nil || !fi.Mode().IsRegular() {
-			// *Only* if tailscaled.log.conf does not exist,
+				// *Only* if tailscaled.log.conf does not exist,
-			// check for tailscale-ipn.log.conf
+				// check for tailscale-ipn.log.conf
-			oldPathOldCmd := filepath.Join(oldDir, "tailscale-ipn.log.conf")
+				oldPathOldCmd := filepath.Join(oldDir, "tailscale-ipn.log.conf")
-			if fi, err := os.Stat(oldPathOldCmd); err == nil && fi.Mode().IsRegular() {
+				if fi, err := os.Stat(oldPathOldCmd); err == nil && fi.Mode().IsRegular() {
-				oldPath = oldPathOldCmd
+					oldPath = oldPathOldCmd
+				}
+			}
+
+			cfgPath = paths.TryConfigFileMigration(earlyLogf, oldPath, cfgPath)
+		case "tailscale-ipn":
+			for _, oldBase := range []string{"wg64.log.conf", "wg32.log.conf"} {
+				oldConf := filepath.Join(dir, oldBase)
+				if fi, err := os.Stat(oldConf); err == nil && fi.Mode().IsRegular() {
+					cfgPath = paths.TryConfigFileMigration(earlyLogf, oldConf, cfgPath)
+					break
+				}
 			}
 		}
-
-		cfgPath = paths.TryConfigFileMigration(oldPath, cfgPath)
 	}
 
 	var oldc *Config

paths/migrate.go

@@ -5,9 +5,10 @@
 package paths
 
 import (
-	"log"
 	"os"
 	"path/filepath"
+
+	"tailscale.com/types/logger"
 )
 
 // TryConfigFileMigration carefully copies the contents of oldFile to
@@ -17,14 +18,14 @@ import (
 //   default config to be written to.
 // - if oldFile exists but copying to newFile fails, return oldFile so
 //   there will at least be some config to work with.
-func TryConfigFileMigration(oldFile, newFile string) string {
+func TryConfigFileMigration(logf logger.Logf, oldFile, newFile string) string {
 	_, err := os.Stat(newFile)
 	if err == nil {
 		// Common case for a system which has already been migrated.
 		return newFile
 	}
 	if !os.IsNotExist(err) {
-		log.Printf("TryConfigFileMigration failed; new file: %v", err)
+		logf("TryConfigFileMigration failed; new file: %v", err)
 		return newFile
 	}
 
@@ -39,15 +40,15 @@ func TryConfigFileMigration(oldFile, newFile string) string {
 	if err != nil {
 		removeErr := os.Remove(newFile)
 		if removeErr != nil {
-			log.Printf("TryConfigFileMigration failed; write newFile no cleanup: %v, remove err: %v",
+			logf("TryConfigFileMigration failed; write newFile no cleanup: %v, remove err: %v",
 				err, removeErr)
 			return oldFile
 		}
-		log.Printf("TryConfigFileMigration failed; write newFile: %v", err)
+		logf("TryConfigFileMigration failed; write newFile: %v", err)
 		return oldFile
 	}
 
-	log.Printf("TryConfigFileMigration: successfully migrated: from %v to %v",
+	logf("TryConfigFileMigration: successfully migrated: from %v to %v",
 		oldFile, newFile)
 
 	return newFile