From 0cb2ccce7fa202e30427eaa28b7a88d92194b0d7 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 28 Nov 2022 20:08:35 -0800 Subject: [PATCH] safesocket: remove the IPN protocol support Updates #6417 Change-Id: I78908633de842d83b2cc8b10a864a0f88ab1b113 Signed-off-by: Brad Fitzpatrick --- client/tailscale/localclient.go | 8 ++-- envknob/envknob.go | 12 +++++ ipn/message.go | 41 ----------------- safesocket/basic_test.go | 2 +- safesocket/safesocket.go | 41 ++++++----------- safesocket/unixsocket.go | 63 +------------------------- tstest/integration/integration_test.go | 1 - version/cmp_test.go | 8 +++- version/export_test.go | 15 ++++++ version/modinfo_test.go | 9 +++- version/version_test.go | 2 +- 11 files changed, 61 insertions(+), 141 deletions(-) delete mode 100644 ipn/message.go create mode 100644 version/export_test.go diff --git a/client/tailscale/localclient.go b/client/tailscale/localclient.go index 9d006303d..e705a9638 100644 --- a/client/tailscale/localclient.go +++ b/client/tailscale/localclient.go @@ -28,6 +28,7 @@ "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 { - onVersionMismatch(ipn.IPCVersion(), server) + if server := res.Header.Get("Tailscale-Version"); server != "" && server != envknob.IPCVersion() && onVersionMismatch != nil { + onVersionMismatch(envknob.IPCVersion(), server) } if res.StatusCode == 403 { all, _ := io.ReadAll(res.Body) diff --git a/envknob/envknob.go b/envknob/envknob.go index 564d29d1c..eb0e9e258 100644 --- a/envknob/envknob.go +++ b/envknob/envknob.go @@ -31,6 +31,7 @@ "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 +} diff --git a/ipn/message.go b/ipn/message.go deleted file mode 100644 index 4ae624e89..000000000 --- a/ipn/message.go +++ /dev/null @@ -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 -} diff --git a/safesocket/basic_test.go b/safesocket/basic_test.go index f3057995f..6d0a09a7f 100644 --- a/safesocket/basic_test.go +++ b/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 diff --git a/safesocket/safesocket.go b/safesocket/safesocket.go index e5b7e80d4..c27834a67 100644 --- a/safesocket/safesocket.go +++ b/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, - // and a flag indicating whether to try fallback connections options. - path string - port uint16 - fallback bool + 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. // @@ -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} -} - -// 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} + return &ConnectionStrategy{path: path, port: WindowsLocalPort} } // Connect connects to tailscaled using s diff --git a/safesocket/unixsocket.go b/safesocket/unixsocket.go index bb80708ca..72649a87e 100644 --- a/safesocket/unixsocket.go +++ b/safesocket/unixsocket.go @@ -9,39 +9,21 @@ 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 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 + return net.Dial("unix", s.path) } -// 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 -} diff --git a/tstest/integration/integration_test.go b/tstest/integration/integration_test.go index 7e616a22b..21f8579b9 100644 --- a/tstest/integration/integration_test.go +++ b/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 { diff --git a/version/cmp_test.go b/version/cmp_test.go index e9abf55ab..b832be3c8 100644 --- a/version/cmp_test.go +++ b/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) } diff --git a/version/export_test.go b/version/export_test.go new file mode 100644 index 000000000..66dddc346 --- /dev/null +++ b/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 +) diff --git a/version/modinfo_test.go b/version/modinfo_test.go index 67df39707..2bf46f6fe 100644 --- a/version/modinfo_test.go +++ b/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 @@ "runtime" "strings" "testing" + + "tailscale.com/version" +) + +var ( + findModuleInfo = version.ExportFindModuleInfo + cmdName = version.ExportCmdName ) func TestFindModuleInfo(t *testing.T) { diff --git a/version/version_test.go b/version/version_test.go index 38a2ae7c2..ca33d8a83 100644 --- a/version/version_test.go +++ b/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"