safesocket: remove ConnectionStrategy (#10662)

This type seems to be a migration shim for TCP tailscaled sockets
(instead of unix/windows pipes). The `port` field was never set, so it
was effectively used as a string (`path` field).
Remove the whole type and simplify call sites to pass the socket path
directly to `safesocket.Connect`.

Updates #cleanup

Signed-off-by: Andrew Lytvynov <awly@tailscale.com>

client/tailscale/localclient.go

@@ -102,8 +102,7 @@ func (lc *LocalClient) defaultDialer(ctx context.Context, network, addr string)
 			return d.DialContext(ctx, "tcp", "127.0.0.1:"+strconv.Itoa(port))
 		}
 	}
-	s := safesocket.DefaultConnectionStrategy(lc.socket())
-	return safesocket.Connect(s)
+	return safesocket.Connect(lc.socket())
 }
 
 // DoLocalRequest makes an HTTP request to the local machine's Tailscale daemon.

logpolicy/logpolicy.go

@@ -714,7 +714,7 @@ func dialContext(ctx context.Context, netw, addr string, netMon *netmon.Monitor,
 	}
 
 	if version.IsWindowsGUI() && strings.HasPrefix(netw, "tcp") {
-		if c, err := safesocket.Connect(safesocket.DefaultConnectionStrategy("")); err == nil {
+		if c, err := safesocket.Connect(""); err == nil {
 			fmt.Fprintf(c, "CONNECT %s HTTP/1.0\r\n\r\n", addr)
 			br := bufio.NewReader(c)
 			res, err := http.ReadResponse(br, nil)

safesocket/basic_test.go

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

safesocket/pipe_windows.go

@@ -17,13 +17,13 @@
 	"golang.org/x/sys/windows"
 )
 
-func connect(s *ConnectionStrategy) (net.Conn, error) {
+func connect(path string) (net.Conn, error) {
 	dl := time.Now().Add(20 * time.Second)
 	ctx, cancel := context.WithDeadline(context.Background(), dl)
 	defer cancel()
 	// We use the identification impersonation level so that tailscaled may
 	// obtain information about our token for access control purposes.
-	return winio.DialPipeAccessImpLevel(ctx, s.path, windows.GENERIC_READ|windows.GENERIC_WRITE, winio.PipeImpLevelIdentification)
+	return winio.DialPipeAccessImpLevel(ctx, path, windows.GENERIC_READ|windows.GENERIC_WRITE, winio.PipeImpLevelIdentification)
 }
 
 func setFlags(network, address string, c syscall.RawConn) error {

safesocket/pipe_windows_test.go

@@ -80,8 +80,7 @@ func TestExpectedWindowsTypes(t *testing.T) {
 	}()
 
 	go func() {
-		s := DefaultConnectionStrategy(sock)
-		c, err := Connect(s)
+		c, err := Connect(sock)
 		if err != nil {
 			errs <- err
 			return

safesocket/safesocket.go

@@ -52,52 +52,10 @@ func tailscaledStillStarting() bool {
 	return tailscaledProcExists()
 }
 
-// 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 {
-	path string // unix socket path
-	port uint16 // TCP port
-
-	// Longer term, a ConnectionStrategy should be an ordered list of things to attempt,
-	// with just the information required to connection for each.
-	//
-	// We have at least these cases to consider (see issue 3530):
-	//
-	//   tailscale sandbox | tailscaled sandbox | OS      | connection
-	//   ------------------|--------------------|---------|-----------
-	//   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
-	//   no                | System Extension   | macOS   | TCP/port/token, port/token from lsof
-	//   yes               | Network Extension  | macOS   | TCP/port/token, port/token from readdir
-	//   yes               | System Extension   | macOS   | TCP/port/token, port/token from readdir
-	//
-	// Note e.g. that port is only relevant as an input to Connect on Windows,
-	// that path is not relevant to Windows, and that neither matters to wasm.
-}
-
-// DefaultConnectionStrategy returns a default connection strategy.
-// The default strategy is to attempt to connect in as many ways as possible.
-// It uses path as the unix socket path, when applicable,
-// and defaults to WindowsLocalPort for the TCP port when applicable.
-// 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}
-}
-
-// Connect connects to tailscaled using s
-func Connect(s *ConnectionStrategy) (net.Conn, error) {
+// Connect connects to tailscaled using a unix socket or named pipe.
+func Connect(path string) (net.Conn, error) {
 	for {
-		c, err := connect(s)
+		c, err := connect(path)
 		if err != nil && tailscaledStillStarting() {
 			time.Sleep(250 * time.Millisecond)
 			continue

safesocket/safesocket_js.go

@@ -15,6 +15,6 @@ func listen(path string) (net.Listener, error) {
 	return memconn.Listen("memu", memName)
 }
 
-func connect(_ *ConnectionStrategy) (net.Conn, error) {
+func connect(_ string) (net.Conn, error) {
 	return memconn.Dial("memu", memName)
 }

safesocket/safesocket_plan9.go

@@ -85,13 +85,13 @@ func (fc plan9FileConn) SetWriteDeadline(t time.Time) error {
 	return syscall.EPLAN9
 }
 
-func connect(s *ConnectionStrategy) (net.Conn, error) {
-	f, err := os.OpenFile(s.path, os.O_RDWR, 0666)
+func connect(path string) (net.Conn, error) {
+	f, err := os.OpenFile(path, os.O_RDWR, 0666)
 	if err != nil {
 		return nil, err
 	}
 
-	return plan9FileConn{name: s.path, file: f}, nil
+	return plan9FileConn{name: path, file: f}, nil
 }
 
 // Create an entry in /srv, open a pipe, write the

safesocket/unixsocket.go

@@ -16,11 +16,11 @@
 	"runtime"
 )
 
-func connect(s *ConnectionStrategy) (net.Conn, error) {
+func connect(path string) (net.Conn, error) {
 	if runtime.GOOS == "js" {
 		return nil, errors.New("safesocket.Connect not yet implemented on js/wasm")
 	}
-	return net.Dial("unix", s.path)
+	return net.Dial("unix", path)
 }
 
 func listen(path string) (net.Listener, error) {

tstest/integration/integration_test.go

@@ -1226,9 +1226,8 @@ func (n *testNode) Ping(otherNode *testNode) error {
 // over its localhost IPC mechanism. (Unix socket, etc)
 func (n *testNode) AwaitListening() {
 	t := n.env.t
-	s := safesocket.DefaultConnectionStrategy(n.sockFile)
 	if err := tstest.WaitFor(20*time.Second, func() (err error) {
-		c, err := safesocket.Connect(s)
+		c, err := safesocket.Connect(n.sockFile)
 		if err == nil {
 			c.Close()
 		}