safesocket, version: fix safesocket_darwin behavior for cmd/tailscale (#15275)

fixes tailscale/tailscale#15269

Fixes the various CLIs for all of the various flavors of tailscaled on
darwin.  The logic in version is updated so that we have methods that
return true only for the actual GUI app (which can beCLI) and the
order of the checks in localTCPPortAndTokenDarwin are corrected so
that the logic works with all 5 combinations of CLI and tailscaled.

Signed-off-by: Jonathan Nobels <jonathan@tailscale.com>

safesocket/safesocket_darwin.go

@@ -34,17 +34,17 @@ type safesocketDarwin struct {
 	mu              sync.Mutex
 	token           string   // safesocket auth token
 	port            int      // safesocket port
-	sameuserproofFD *os.File // file descriptor for macos app store sameuserproof file
+	sameuserproofFD *os.File // File descriptor for macos app store sameuserproof file
-	sharedDir       string   // shared directory for location of sameuserproof file
+	sharedDir       string   // Shared directory for location of sameuserproof file
 
-	checkConn   bool        // Check macsys safesocket port before returning it
+	checkConn   bool        // If true, check macsys safesocket port before returning it
-	isMacSysExt func() bool // For testing only to force macsys
+	isMacSysExt func() bool // Reports true if this binary is the macOS System Extension
-	isMacGUIApp func() bool // For testing only to force macOS sandbox
+	isMacGUIApp func() bool // Reports true if running as a macOS GUI app (Tailscale.app)
 }
 
 var ssd = safesocketDarwin{
 	isMacSysExt: version.IsMacSysExt,
-	isMacGUIApp: func() bool { return version.IsMacAppStore() || version.IsMacSysApp() || version.IsMacSysExt() },
+	isMacGUIApp: func() bool { return version.IsMacAppStoreGUI() || version.IsMacSysGUI() },
 	checkConn:   true,
 	sharedDir:   "/Library/Tailscale",
 }
@@ -63,22 +63,25 @@ var ssd = safesocketDarwin{
 // calls InitListenerDarwin.
 
 // localTCPPortAndTokenDarwin returns the localhost TCP port number and auth token
-// either generated, or sourced from the NEPacketTunnelProvider managed tailscaled process.
+// either from the sameuserproof mechanism, or source and set directly from the
+// NEPacketTunnelProvider managed tailscaled process when the CLI is invoked
+// from the Tailscale.app GUI.
 func localTCPPortAndTokenDarwin() (port int, token string, err error) {
 	ssd.mu.Lock()
 	defer ssd.mu.Unlock()
 
-	if !ssd.isMacGUIApp() {
+	switch {
-		return 0, "", ErrNoTokenOnOS
+	case ssd.port != 0 && ssd.token != "":
-	}
+		// If something has explicitly set our credentials (typically non-standalone macos binary), use them.
-
-	if ssd.port != 0 && ssd.token != "" {
 		return ssd.port, ssd.token, nil
-	}
+	case !ssd.isMacGUIApp():
-
+		// We're not a GUI app (probably cmd/tailscale), so try falling back to sameuserproof.
-	// Credentials were not explicitly, this is likely a standalone CLI binary.
+		// If portAndTokenFromSameUserProof returns an error here, cmd/tailscale will
-	// Fallback to reading the sameuserproof file.
+		// attempt to use the default unix socket mechanism supported by tailscaled.
 		return portAndTokenFromSameUserProof()
+	default:
+		return 0, "", ErrTokenNotFound
+	}
 }
 
 // SetCredentials sets an token and port used to authenticate safesocket generated
@@ -341,6 +344,11 @@ func readMacosSameUserProof() (port int, token string, err error) {
 }
 
 func portAndTokenFromSameUserProof() (port int, token string, err error) {
+	// When we're cmd/tailscale, we have no idea what tailscaled is, so we'll try
+	// macos, then macsys and finally, fallback to tailscaled via a unix socket
+	// if both of those return an error.   You can run macos or macsys and
+	// tailscaled at the same time, but we are forced to choose one and the GUI
+	// clients are first in line here.  You cannot run macos and macsys simultaneously.
 	if port, token, err := readMacosSameUserProof(); err == nil {
 		return port, token, nil
 	}
@@ -349,5 +357,5 @@ func portAndTokenFromSameUserProof() (port int, token string, err error) {
 		return port, token, nil
 	}
 
-	return 0, "", err
+	return 0, "", ErrTokenNotFound
 }

safesocket/safesocket_darwin_test.go

@@ -15,9 +15,12 @@ import (
 // sets the port and token correctly and that LocalTCPPortAndToken
 // returns the given values.
 func TestSetCredentials(t *testing.T) {
-	wantPort := 123
+	const (
-	wantToken := "token"
+		wantToken = "token"
-	tstest.Replace(t, &ssd.isMacGUIApp, func() bool { return true })
+		wantPort  = 123
+	)
+
+	tstest.Replace(t, &ssd.isMacGUIApp, func() bool { return false })
 	SetCredentials(wantToken, wantPort)
 
 	gotPort, gotToken, err := LocalTCPPortAndToken()
@@ -26,11 +29,47 @@ func TestSetCredentials(t *testing.T) {
 	}
 
 	if gotPort != wantPort {
-		t.Errorf("got port %d, want %d", gotPort, wantPort)
+		t.Errorf("port: got %d, want %d", gotPort, wantPort)
 	}
 
 	if gotToken != wantToken {
-		t.Errorf("got token %s, want %s", gotToken, wantToken)
+		t.Errorf("token: got %s, want %s", gotToken, wantToken)
+	}
+}
+
+// TestFallbackToSameuserproof verifies that we fallback to the
+// sameuserproof file via LocalTCPPortAndToken when we're running
+//
+//	s cmd/tailscale
+func TestFallbackToSameuserproof(t *testing.T) {
+	dir := t.TempDir()
+	const (
+		wantToken = "token"
+		wantPort  = 123
+	)
+
+	// Mimics cmd/tailscale falling back to sameuserproof
+	tstest.Replace(t, &ssd.isMacGUIApp, func() bool { return false })
+	tstest.Replace(t, &ssd.sharedDir, dir)
+	tstest.Replace(t, &ssd.checkConn, false)
+
+	// Behave as macSysExt when initializing sameuserproof
+	tstest.Replace(t, &ssd.isMacSysExt, func() bool { return true })
+	if err := initSameUserProofToken(dir, wantPort, wantToken); err != nil {
+		t.Fatalf("initSameUserProofToken: %v", err)
+	}
+
+	gotPort, gotToken, err := LocalTCPPortAndToken()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	if gotPort != wantPort {
+		t.Errorf("port: got %d, want %d", gotPort, wantPort)
+	}
+
+	if gotToken != wantToken {
+		t.Errorf("token: got %s, want %s", gotToken, wantToken)
 	}
 }
 
@@ -38,7 +77,7 @@ func TestSetCredentials(t *testing.T) {
 // returns a listener and a non-zero port and non-empty token.
 func TestInitListenerDarwin(t *testing.T) {
 	temp := t.TempDir()
-	tstest.Replace(t, &ssd.isMacGUIApp, func() bool { return true })
+	tstest.Replace(t, &ssd.isMacGUIApp, func() bool { return false })
 
 	ln, err := InitListenerDarwin(temp)
 	if err != nil || ln == nil {
@@ -52,15 +91,14 @@ func TestInitListenerDarwin(t *testing.T) {
 	}
 
 	if port == 0 {
-		t.Errorf("expected non-zero port, got %d", port)
+		t.Errorf("port: got %d, want non-zero", port)
 	}
 
 	if token == "" {
-		t.Errorf("expected non-empty token, got empty string")
+		t.Errorf("token: got %s, want non-empty", token)
 	}
 }
 
-// TestTokenGeneration verifies token generation behavior
 func TestTokenGeneration(t *testing.T) {
 	token, err := getToken()
 	if err != nil {
@@ -70,7 +108,7 @@ func TestTokenGeneration(t *testing.T) {
 	// Verify token length (hex string is 2x byte length)
 	wantLen := sameUserProofTokenLength * 2
 	if got := len(token); got != wantLen {
-		t.Errorf("token length = %d, want %d", got, wantLen)
+		t.Errorf("token length: got %d, want %d", got, wantLen)
 	}
 
 	// Verify token persistence
@@ -79,7 +117,7 @@ func TestTokenGeneration(t *testing.T) {
 		t.Fatalf("subsequent getToken: %v", err)
 	}
 	if subsequentToken != token {
-		t.Errorf("subsequent token = %q, want %q", subsequentToken, token)
+		t.Errorf("subsequent token: got %q, want %q", subsequentToken, token)
 	}
 }
 
@@ -107,10 +145,10 @@ func TestMacsysSameuserproof(t *testing.T) {
 	}
 
 	if gotPort != wantPort {
-		t.Errorf("got port = %d, want %d", gotPort, wantPort)
+		t.Errorf("port: got %d, want %d", gotPort, wantPort)
 	}
 	if wantToken != gotToken {
-		t.Errorf("got token = %s, want %s", wantToken, gotToken)
+		t.Errorf("token: got %s, want %s", wantToken, gotToken)
 	}
 	assertFileCount(t, dir, 1, "sameuserproof-")
 }
@@ -138,7 +176,7 @@ func assertFileCount(t *testing.T, dir string, want int, prefix string) {
 
 	files, err := os.ReadDir(dir)
 	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
+		t.Fatalf("[unexpected] error: %v", err)
 	}
 	count := 0
 	for _, file := range files {
@@ -147,6 +185,6 @@ func assertFileCount(t *testing.T, dir string, want int, prefix string) {
 		}
 	}
 	if count != want {
-		t.Errorf("expected 1 file, got %d", count)
+		t.Errorf("files: got %d, want 1", count)
 	}
 }

version/prop.go

@@ -62,26 +62,21 @@ func IsSandboxedMacOS() bool {
 // Tailscale for macOS, either the main GUI process (non-sandboxed) or the
 // system extension (sandboxed).
 func IsMacSys() bool {
-	return IsMacSysExt() || IsMacSysApp()
+	return IsMacSysExt() || IsMacSysGUI()
 }
 
 var isMacSysApp lazy.SyncValue[bool]
 
-// IsMacSysApp reports whether this process is the main, non-sandboxed GUI process
+// IsMacSysGUI reports whether this process is the main, non-sandboxed GUI process
 // that ships with the Standalone variant of Tailscale for macOS.
-func IsMacSysApp() bool {
+func IsMacSysGUI() bool {
 	if runtime.GOOS != "darwin" {
 		return false
 	}
 
 	return isMacSysApp.Get(func() bool {
-		exe, err := os.Executable()
+		return strings.Contains(os.Getenv("HOME"), "/Containers/io.tailscale.ipn.macsys/") ||
-		if err != nil {
+			strings.Contains(os.Getenv("XPC_SERVICE_NAME"), "io.tailscale.ipn.macsys")
-			return false
-		}
-		// Check that this is the GUI binary, and it is not sandboxed. The GUI binary
-		// shipped in the App Store will always have the App Sandbox enabled.
-		return strings.HasSuffix(exe, "/Contents/MacOS/Tailscale") && !IsMacAppStore()
 	})
 }
 
@@ -95,10 +90,6 @@ func IsMacSysExt() bool {
 		return false
 	}
 	return isMacSysExt.Get(func() bool {
-		if strings.Contains(os.Getenv("HOME"), "/Containers/io.tailscale.ipn.macsys/") ||
-			strings.Contains(os.Getenv("XPC_SERVICE_NAME"), "io.tailscale.ipn.macsys") {
-			return true
-		}
 		exe, err := os.Executable()
 		if err != nil {
 			return false
@@ -109,8 +100,8 @@ func IsMacSysExt() bool {
 
 var isMacAppStore lazy.SyncValue[bool]
 
-// IsMacAppStore whether this binary is from the App Store version of Tailscale
+// IsMacAppStore returns whether this binary is from the App Store version of Tailscale
-// for macOS.
+// for macOS.  Returns true for both the network extension and the GUI app.
 func IsMacAppStore() bool {
 	if runtime.GOOS != "darwin" {
 		return false
@@ -124,6 +115,25 @@ func IsMacAppStore() bool {
 	})
 }
 
+var isMacAppStoreGUI lazy.SyncValue[bool]
+
+// IsMacAppStoreGUI reports whether this binary is the GUI app from the App Store
+// version of Tailscale for macOS.
+func IsMacAppStoreGUI() bool {
+	if runtime.GOOS != "darwin" {
+		return false
+	}
+	return isMacAppStoreGUI.Get(func() bool {
+		exe, err := os.Executable()
+		if err != nil {
+			return false
+		}
+		// Check that this is the GUI binary, and it is not sandboxed. The GUI binary
+		// shipped in the App Store will always have the App Sandbox enabled.
+		return strings.Contains(exe, "/Tailscale") && !IsMacSysGUI()
+	})
+}
+
 var isAppleTV lazy.SyncValue[bool]
 
 // IsAppleTV reports whether this binary is part of the Tailscale network extension for tvOS.