safesocket: remove the IPN protocol support

Updates #6417

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

client/tailscale/localclient.go

@@ -28,6 +28,7 @@ import (
 
 	"go4.org/mem"
 	"tailscale.com/client/tailscale/apitype"
+	"tailscale.com/envknob"
 	"tailscale.com/ipn"
 	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/net/netutil"
@@ -100,9 +101,6 @@ func (lc *LocalClient) defaultDialer(ctx context.Context, network, addr string)
 		}
 	}
 	s := safesocket.DefaultConnectionStrategy(lc.socket())
-	// The user provided a non-default tailscaled socket address.
-	// Connect only to exactly what they provided.
-	s.UseFallback(false)
 	return safesocket.Connect(s)
 }
 
@@ -132,8 +130,8 @@ func (lc *LocalClient) DoLocalRequest(req *http.Request) (*http.Response, error)
 func (lc *LocalClient) doLocalRequestNiceError(req *http.Request) (*http.Response, error) {
 	res, err := lc.DoLocalRequest(req)
 	if err == nil {
-		if server := res.Header.Get("Tailscale-Version"); server != "" && server != ipn.IPCVersion() && onVersionMismatch != nil {
+		if server := res.Header.Get("Tailscale-Version"); server != "" && server != envknob.IPCVersion() && onVersionMismatch != nil {
-			onVersionMismatch(ipn.IPCVersion(), server)
+			onVersionMismatch(envknob.IPCVersion(), server)
 		}
 		if res.StatusCode == 403 {
 			all, _ := io.ReadAll(res.Body)

envknob/envknob.go

@@ -31,6 +31,7 @@ import (
 	"sync/atomic"
 
 	"tailscale.com/types/opt"
+	"tailscale.com/version"
 	"tailscale.com/version/distro"
 )
 
@@ -432,3 +433,14 @@ func applyKeyValueEnv(r io.Reader) error {
 	}
 	return bs.Err()
 }
+
+// IPCVersion returns version.Long usually, unless TS_DEBUG_FAKE_IPC_VERSION is
+// set, in which it contains that value. This is only used for weird development
+// cases when testing mismatched versions and you want the client to act like it's
+// compatible with the server.
+func IPCVersion() string {
+	if v := String("TS_DEBUG_FAKE_IPC_VERSION"); v != "" {
+		return v
+	}
+	return version.Long
+}

ipn/message.go

@@ -1,41 +0,0 @@
-// Copyright (c) 2020 Tailscale Inc & AUTHORS All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-package ipn
-
-import (
-	"context"
-
-	"tailscale.com/envknob"
-	"tailscale.com/version"
-)
-
-type readOnlyContextKey struct{}
-
-// IsReadonlyContext reports whether ctx is a read-only context, as currently used
-// by Unix non-root users running the "tailscale" CLI command. They can run "status",
-// but not much else.
-func IsReadonlyContext(ctx context.Context) bool {
-	return ctx.Value(readOnlyContextKey{}) != nil
-}
-
-// ReadonlyContextOf returns ctx wrapped with a context value that
-// will make IsReadonlyContext reports true.
-func ReadonlyContextOf(ctx context.Context) context.Context {
-	if IsReadonlyContext(ctx) {
-		return ctx
-	}
-	return context.WithValue(ctx, readOnlyContextKey{}, readOnlyContextKey{})
-}
-
-// IPCVersion returns version.Long usually, unless TS_DEBUG_FAKE_IPC_VERSION is
-// set, in which it contains that value. This is only used for weird development
-// cases when testing mismatched versions and you want the client to act like it's
-// compatible with the server.
-func IPCVersion() string {
-	if v := envknob.String("TS_DEBUG_FAKE_IPC_VERSION"); v != "" {
-		return v
-	}
-	return version.Long
-}

safesocket/basic_test.go

@@ -49,7 +49,7 @@ func TestBasics(t *testing.T) {
 
 	go func() {
 		s := DefaultConnectionStrategy(sock)
-		s.UsePort(port)
+		s.port = port
 		c, err := Connect(s)
 		if err != nil {
 			errs <- err

safesocket/safesocket.go

@@ -57,13 +57,19 @@ func tailscaledStillStarting() bool {
 	return tailscaledProcExists()
 }
 
-// A ConnectionStrategy is a plan for how to connect to tailscaled or equivalent (e.g. IPNExtension on macOS).
+// A ConnectionStrategy is a plan for how to connect to tailscaled or equivalent
+// (e.g. IPNExtension on macOS).
+//
+// This is a struct because prior to Tailscale 1.34.0 it was more complicated
+// and there were multiple protocols that could be used. See LocalClient's
+// dialer for what happens in practice these days (2022-11-28).
+//
+// TODO(bradfitz): we can remove this struct now and revert this package closer
+// to its original smaller API.
 type ConnectionStrategy struct {
-	// For now, a ConnectionStrategy is just a unix socket path, a TCP port,
+	path string // unix socket path
-	// and a flag indicating whether to try fallback connections options.
+	port uint16 // TCP port
-	path     string
+
-	port     uint16
-	fallback bool
 	// Longer term, a ConnectionStrategy should be an ordered list of things to attempt,
 	// with just the information required to connection for each.
 	//
@@ -71,7 +77,7 @@ type ConnectionStrategy struct {
 	//
 	//   tailscale sandbox | tailscaled sandbox | OS      | connection
 	//   ------------------|--------------------|---------|-----------
-	//   no                | no                 | unix    | unix socket
+	//   no                | no                 | unix*   | unix socket *includes tailscaled on darwin
 	//   no                | no                 | Windows | TCP/port
 	//   no                | no                 | wasm    | memconn
 	//   no                | Network Extension  | macOS   | TCP/port/token, port/token from lsof
@@ -90,26 +96,7 @@ type ConnectionStrategy struct {
 // It falls back to auto-discovery across sandbox boundaries on macOS.
 // TODO: maybe take no arguments, since path is irrelevant on Windows? Discussion in PR 3499.
 func DefaultConnectionStrategy(path string) *ConnectionStrategy {
-	return &ConnectionStrategy{path: path, port: WindowsLocalPort, fallback: true}
+	return &ConnectionStrategy{path: path, port: WindowsLocalPort}
-}
-
-// UsePort modifies s to use port for the TCP port when applicable.
-// UsePort is only applicable on Windows, and only then
-// when not using the default for Windows.
-func (s *ConnectionStrategy) UsePort(port uint16) {
-	s.port = port
-}
-
-// UseFallback modifies s to set whether it should fall back
-// to connecting to the macOS GUI's tailscaled
-// if the Unix socket path wasn't reachable.
-func (s *ConnectionStrategy) UseFallback(b bool) {
-	s.fallback = b
-}
-
-// ExactPath returns a connection strategy that only attempts to connect via path.
-func ExactPath(path string) *ConnectionStrategy {
-	return &ConnectionStrategy{path: path, fallback: false}
 }
 
 // Connect connects to tailscaled using s

safesocket/unixsocket.go

@@ -9,39 +9,21 @@ package safesocket
 import (
 	"errors"
 	"fmt"
-	"io"
 	"log"
 	"net"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"runtime"
-	"strconv"
 )
 
-// TODO(apenwarr): handle magic cookie auth
 func connect(s *ConnectionStrategy) (net.Conn, error) {
 	if runtime.GOOS == "js" {
 		return nil, errors.New("safesocket.Connect not yet implemented on js/wasm")
 	}
-	if runtime.GOOS == "darwin" && s.fallback && s.path == "" && s.port == 0 {
+	return net.Dial("unix", s.path)
-		return connectMacOSAppSandbox()
-	}
-	pipe, err := net.Dial("unix", s.path)
-	if err != nil {
-		if runtime.GOOS == "darwin" && s.fallback {
-			extConn, extErr := connectMacOSAppSandbox()
-			if extErr != nil {
-				return nil, fmt.Errorf("safesocket: failed to connect to %v: %v; failed to connect to Tailscale IPNExtension: %v", s.path, err, extErr)
-			}
-			return extConn, nil
-		}
-		return nil, err
-	}
-	return pipe, nil
 }
 
-// TODO(apenwarr): handle magic cookie auth
 func listen(path string, port uint16) (ln net.Listener, _ uint16, err error) {
 	// Unix sockets hang around in the filesystem even after nobody
 	// is listening on them. (Which is really unfortunate but long-
@@ -110,46 +92,3 @@ func socketPermissionsForOS() os.FileMode {
 	// Otherwise, root only.
 	return 0600
 }
-
-// connectMacOSAppSandbox connects to the Tailscale Network Extension (macOS App
-// Store build) or App Extension (macsys standalone build), where the CLI itself
-// is either running within the macOS App Sandbox or built separately (e.g.
-// homebrew or go install). This little dance to connect a regular user binary
-// to the sandboxed network extension is:
-//
-//   - the sandboxed IPNExtension picks a random localhost:0 TCP port
-//     to listen on
-//   - it also picks a random hex string that acts as an auth token
-//   - the CLI looks on disk for that TCP port + auth token (see localTCPPortAndTokenDarwin)
-//   - we send it upon TCP connect to prove to the Tailscale daemon that
-//     we're a suitably privileged user to have access the files on disk
-//     which the Network/App Extension wrote.
-func connectMacOSAppSandbox() (net.Conn, error) {
-	port, token, err := LocalTCPPortAndToken()
-	if err != nil {
-		return nil, fmt.Errorf("failed to find local Tailscale daemon: %w", err)
-	}
-	return connectMacTCP(port, token)
-}
-
-// connectMacTCP creates an authenticated net.Conn to the local macOS Tailscale
-// daemon for used by the "IPN" JSON message bus protocol (Tailscale's original
-// local non-HTTP IPC protocol).
-func connectMacTCP(port int, token string) (net.Conn, error) {
-	c, err := net.Dial("tcp", "localhost:"+strconv.Itoa(port))
-	if err != nil {
-		return nil, fmt.Errorf("error dialing IPNExtension: %w", err)
-	}
-	if _, err := io.WriteString(c, token+"\n"); err != nil {
-		return nil, fmt.Errorf("error writing auth token: %w", err)
-	}
-	buf := make([]byte, 5)
-	const authOK = "#IPN\n"
-	if _, err := io.ReadFull(c, buf); err != nil {
-		return nil, fmt.Errorf("error reading from IPNExtension post-auth: %w", err)
-	}
-	if string(buf) != authOK {
-		return nil, fmt.Errorf("invalid response reading from IPNExtension post-auth")
-	}
-	return c, nil
-}

tstest/integration/integration_test.go

@@ -871,7 +871,6 @@ func (n *testNode) Ping(otherNode *testNode) error {
 func (n *testNode) AwaitListening() {
 	t := n.env.t
 	s := safesocket.DefaultConnectionStrategy(n.sockFile)
-	s.UseFallback(false) // connect only to the tailscaled that we started
 	if err := tstest.WaitFor(20*time.Second, func() (err error) {
 		c, err := safesocket.Connect(s)
 		if err != nil {

version/cmp_test.go

@@ -2,16 +2,20 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-package version
+package version_test
 
 import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
 	"tailscale.com/tstest"
+	"tailscale.com/version"
 )
 
 func TestParse(t *testing.T) {
+	parse := version.ExportParse
+	type parsed = version.ExportParsed
+
 	tests := []struct {
 		version string
 		parsed  parsed
@@ -71,7 +75,7 @@ func TestAtLeast(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		got := AtLeast(test.v, test.m)
+		got := version.AtLeast(test.v, test.m)
 		if got != test.want {
 			t.Errorf("AtLeast(%q, %q) = %v, want %v", test.v, test.m, got, test.want)
 		}

version/export_test.go

@@ -0,0 +1,15 @@
+// Copyright (c) 2022 Tailscale Inc & AUTHORS All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package version
+
+var (
+	ExportParse          = parse
+	ExportFindModuleInfo = findModuleInfo
+	ExportCmdName        = cmdName
+)
+
+type (
+	ExportParsed = parsed
+)

version/modinfo_test.go

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-package version
+package version_test
 
 import (
 	"flag"
@@ -11,6 +11,13 @@ import (
 	"runtime"
 	"strings"
 	"testing"
+
+	"tailscale.com/version"
+)
+
+var (
+	findModuleInfo = version.ExportFindModuleInfo
+	cmdName        = version.ExportCmdName
 )
 
 func TestFindModuleInfo(t *testing.T) {

version/version_test.go

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-package version
+package version_test
 
 import (
 	"bytes"