Browse Source

clientupdate, util/osshare, util/winutil, version: improve Windows GUI filename resolution and WinUI build awareness

On Windows arm64 we are going to need to ship two different GUI builds;
one for Win10 (GOARCH=386) and one for Win11 (GOARCH=amd64, tags +=
winui). Due to quirks in MSI packaging, they cannot both share the
same filename. This requires some fixes in places where we have
hardcoded "tailscale-ipn" as the GUI filename.

We also do some cleanup in clientupdate to ensure that autoupdates
will continue to work correctly with the temporary "-winui" package
variant.

Fixes #17480
Updates https://github.com/tailscale/corp/issues/29940

Signed-off-by: Aaron Klotz <[email protected]>
Aaron Klotz 5 months ago
parent
commit
7c49cab1a6

+ 23 - 9
clientupdate/clientupdate_windows.go

@@ -30,11 +30,6 @@ const (
 	// tailscale.exe process from running before the msiexec process runs and
 	// tries to overwrite ourselves.
 	winMSIEnv = "TS_UPDATE_WIN_MSI"
-	// winExePathEnv is the environment variable that is set along with
-	// winMSIEnv and carries the full path of the calling tailscale.exe binary.
-	// It is used to re-launch the GUI process (tailscale-ipn.exe) after
-	// install is complete.
-	winExePathEnv = "TS_UPDATE_WIN_EXE_PATH"
 	// winVersionEnv is the environment variable that is set along with
 	// winMSIEnv and carries the version of tailscale that is being installed.
 	// It is used for logging purposes.
@@ -78,6 +73,17 @@ func verifyAuthenticode(path string) error {
 	return authenticode.Verify(path, certSubjectTailscale)
 }
 
+func isTSGUIPresent() bool {
+	us, err := os.Executable()
+	if err != nil {
+		return false
+	}
+
+	tsgui := filepath.Join(filepath.Dir(us), "tsgui.dll")
+	_, err = os.Stat(tsgui)
+	return err == nil
+}
+
 func (up *Updater) updateWindows() error {
 	if msi := os.Getenv(winMSIEnv); msi != "" {
 		// stdout/stderr from this part of the install could be lost since the
@@ -131,7 +137,15 @@ you can run the command prompt as Administrator one of these ways:
 		return err
 	}
 	up.cleanupOldDownloads(filepath.Join(msiDir, "*.msi"))
-	pkgsPath := fmt.Sprintf("%s/tailscale-setup-%s-%s.msi", up.Track, ver, arch)
+
+	qualifiers := []string{ver, arch}
+	// TODO(aaron): Temporary hack so autoupdate still works on winui builds;
+	// remove when we enable winui by default on the unstable track.
+	if isTSGUIPresent() {
+		qualifiers = append(qualifiers, "winui")
+	}
+
+	pkgsPath := fmt.Sprintf("%s/tailscale-setup-%s.msi", up.Track, strings.Join(qualifiers, "-"))
 	msiTarget := filepath.Join(msiDir, path.Base(pkgsPath))
 	if err := up.downloadURLToFile(pkgsPath, msiTarget); err != nil {
 		return err
@@ -145,7 +159,7 @@ you can run the command prompt as Administrator one of these ways:
 
 	up.Logf("making tailscale.exe copy to switch to...")
 	up.cleanupOldDownloads(filepath.Join(os.TempDir(), updaterPrefix+"-*.exe"))
-	selfOrig, selfCopy, err := makeSelfCopy()
+	_, selfCopy, err := makeSelfCopy()
 	if err != nil {
 		return err
 	}
@@ -153,7 +167,7 @@ you can run the command prompt as Administrator one of these ways:
 	up.Logf("running tailscale.exe copy for final install...")
 
 	cmd := exec.Command(selfCopy, "update")
-	cmd.Env = append(os.Environ(), winMSIEnv+"="+msiTarget, winExePathEnv+"="+selfOrig, winVersionEnv+"="+ver)
+	cmd.Env = append(os.Environ(), winMSIEnv+"="+msiTarget, winVersionEnv+"="+ver)
 	cmd.Stdout = up.Stderr
 	cmd.Stderr = up.Stderr
 	cmd.Stdin = os.Stdin
@@ -189,7 +203,7 @@ func (up *Updater) installMSI(msi string) error {
 			case windows.ERROR_SUCCESS_REBOOT_REQUIRED:
 				// In most cases, updating Tailscale should not require a reboot.
 				// If it does, it might be because we failed to close the GUI
-				// and the installer couldn't replace tailscale-ipn.exe.
+				// and the installer couldn't replace its executable.
 				// The old GUI will continue to run until the next reboot.
 				// Not ideal, but also not a retryable error.
 				up.Logf("[unexpected] reboot required")

+ 29 - 17
util/osshare/filesharingstatus_windows.go

@@ -9,30 +9,31 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"sync"
+	"runtime"
 
 	"golang.org/x/sys/windows/registry"
+	"tailscale.com/types/lazy"
 	"tailscale.com/types/logger"
+	"tailscale.com/util/winutil"
 )
 
 const (
 	sendFileShellKey = `*\shell\tailscale`
 )
 
-var ipnExePath struct {
-	sync.Mutex
-	cache string // absolute path of tailscale-ipn.exe, populated lazily on first use
-}
+var ipnExePath lazy.SyncValue[string] // absolute path of the GUI executable
 
 func getIpnExePath(logf logger.Logf) string {
-	ipnExePath.Lock()
-	defer ipnExePath.Unlock()
-
-	if ipnExePath.cache != "" {
-		return ipnExePath.cache
+	exe, err := winutil.GUIPathFromReg()
+	if err == nil {
+		return exe
 	}
 
-	// Find the absolute path of tailscale-ipn.exe assuming that it's in the same
+	return findGUIInSameDirAsThisExe(logf)
+}
+
+func findGUIInSameDirAsThisExe(logf logger.Logf) string {
+	// Find the absolute path of the GUI, assuming that it's in the same
 	// directory as this executable (tailscaled.exe).
 	p, err := os.Executable()
 	if err != nil {
@@ -43,14 +44,23 @@ func getIpnExePath(logf logger.Logf) string {
 		logf("filepath.EvalSymlinks error: %v", err)
 		return ""
 	}
-	p = filepath.Join(filepath.Dir(p), "tailscale-ipn.exe")
 	if p, err = filepath.Abs(p); err != nil {
 		logf("filepath.Abs error: %v", err)
 		return ""
 	}
-	ipnExePath.cache = p
-
-	return p
+	d := filepath.Dir(p)
+	candidates := []string{"tailscale-ipn.exe"}
+	if runtime.GOARCH == "arm64" {
+		// This name may be used on Windows 10 ARM64.
+		candidates = append(candidates, "tailscale-gui-386.exe")
+	}
+	for _, c := range candidates {
+		testPath := filepath.Join(d, c)
+		if _, err := os.Stat(testPath); err == nil {
+			return testPath
+		}
+	}
+	return ""
 }
 
 // SetFileSharingEnabled adds/removes "Send with Tailscale" from the Windows shell menu.
@@ -64,7 +74,9 @@ func SetFileSharingEnabled(enabled bool, logf logger.Logf) {
 }
 
 func enableFileSharing(logf logger.Logf) {
-	path := getIpnExePath(logf)
+	path := ipnExePath.Get(func() string {
+		return getIpnExePath(logf)
+	})
 	if path == "" {
 		return
 	}
@@ -79,7 +91,7 @@ func enableFileSharing(logf logger.Logf) {
 		logf("k.SetStringValue error: %v", err)
 		return
 	}
-	if err := k.SetStringValue("Icon", path+",0"); err != nil {
+	if err := k.SetStringValue("Icon", path+",1"); err != nil {
 		logf("k.SetStringValue error: %v", err)
 		return
 	}

+ 25 - 0
util/winutil/winutil_windows.go

@@ -8,8 +8,10 @@ import (
 	"fmt"
 	"log"
 	"math"
+	"os"
 	"os/exec"
 	"os/user"
+	"path/filepath"
 	"reflect"
 	"runtime"
 	"strings"
@@ -33,6 +35,10 @@ var ErrNoShell = errors.New("no Shell process is present")
 // ErrNoValue is returned when the value doesn't exist in the registry.
 var ErrNoValue = registry.ErrNotExist
 
+// ErrBadRegValueFormat is returned when a string value does not match the
+// expected format.
+var ErrBadRegValueFormat = errors.New("registry value formatted incorrectly")
+
 // GetDesktopPID searches the PID of the process that's running the
 // currently active desktop. Returns ErrNoShell if the shell is not present.
 // Usually the PID will be for explorer.exe.
@@ -947,3 +953,22 @@ func IsDomainName(name string) (bool, error) {
 
 	return isDomainName(name16)
 }
+
+// GUIPathFromReg obtains the path to the client GUI executable from the
+// registry value that was written during installation.
+func GUIPathFromReg() (string, error) {
+	regPath, err := GetRegString("GUIPath")
+	if err != nil {
+		return "", err
+	}
+
+	if !filepath.IsAbs(regPath) {
+		return "", ErrBadRegValueFormat
+	}
+
+	if _, err := os.Stat(regPath); err != nil {
+		return "", err
+	}
+
+	return regPath, nil
+}

+ 6 - 6
version/cmdname.go

@@ -12,7 +12,7 @@ import (
 	"io"
 	"os"
 	"path"
-	"path/filepath"
+	"runtime"
 	"strings"
 )
 
@@ -30,7 +30,7 @@ func CmdName() string {
 func cmdName(exe string) string {
 	// fallbackName, the lowercase basename of the executable, is what we return if
 	// we can't find the Go module metadata embedded in the file.
-	fallbackName := filepath.Base(strings.TrimSuffix(strings.ToLower(exe), ".exe"))
+	fallbackName := prepExeNameForCmp(exe, runtime.GOARCH)
 
 	var ret string
 	info, err := findModuleInfo(exe)
@@ -45,10 +45,10 @@ func cmdName(exe string) string {
 			break
 		}
 	}
-	if strings.HasPrefix(ret, "wg") && fallbackName == "tailscale-ipn" {
-		// The tailscale-ipn.exe binary for internal build system packaging reasons
-		// has a path of "tailscale.io/win/wg64", "tailscale.io/win/wg32", etc.
-		// Ignore that name and use "tailscale-ipn" instead.
+	if runtime.GOOS == "windows" && strings.HasPrefix(ret, "gui") && checkPreppedExeNameForGUI(fallbackName) {
+		// The GUI binary for internal build system packaging reasons
+		// has a path of "tailscale.io/win/gui".
+		// Ignore that name and use fallbackName instead.
 		return fallbackName
 	}
 	if ret == "" {

+ 25 - 0
version/exename.go

@@ -0,0 +1,25 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+package version
+
+import (
+	"path/filepath"
+	"strings"
+)
+
+// prepExeNameForCmp strips any extension and arch suffix from exe, and
+// lowercases it.
+func prepExeNameForCmp(exe, arch string) string {
+	baseNoExt := strings.ToLower(strings.TrimSuffix(filepath.Base(exe), filepath.Ext(exe)))
+	archSuffix := "-" + arch
+	return strings.TrimSuffix(baseNoExt, archSuffix)
+}
+
+func checkPreppedExeNameForGUI(preppedExeName string) bool {
+	return preppedExeName == "tailscale-ipn" || preppedExeName == "tailscale-gui"
+}
+
+func isGUIExeName(exe, arch string) bool {
+	return checkPreppedExeNameForGUI(prepExeNameForCmp(exe, arch))
+}

+ 3 - 1
version/prop.go

@@ -159,7 +159,9 @@ func IsWindowsGUI() bool {
 		if err != nil {
 			return false
 		}
-		return strings.EqualFold(exe, "tailscale-ipn.exe") || strings.EqualFold(exe, "tailscale-ipn")
+		// It is okay to use GOARCH here because we're checking whether our
+		// _own_ process is the GUI.
+		return isGUIExeName(exe, runtime.GOARCH)
 	})
 }
 

+ 35 - 0
version/version_internal_test.go

@@ -25,3 +25,38 @@ func TestIsValidLongWithTwoRepos(t *testing.T) {
 		}
 	}
 }
+
+func TestPrepExeNameForCmp(t *testing.T) {
+	cases := []struct {
+		exe  string
+		want string
+	}{
+		{
+			"tailscale-ipn.exe",
+			"tailscale-ipn",
+		},
+		{
+			"tailscale-gui-amd64.exe",
+			"tailscale-gui",
+		},
+		{
+			"tailscale-gui-amd64",
+			"tailscale-gui",
+		},
+		{
+			"tailscale-ipn",
+			"tailscale-ipn",
+		},
+		{
+			"TaIlScAlE-iPn.ExE",
+			"tailscale-ipn",
+		},
+	}
+
+	for _, c := range cases {
+		got := prepExeNameForCmp(c.exe, "amd64")
+		if got != c.want {
+			t.Errorf("prepExeNameForCmp(%q) = %q; want %q", c.exe, got, c.want)
+		}
+	}
+}