control/controlhttp/controlhttpserver: split out Accept to its own package

Otherwise all the clients only using control/controlhttp for the
ts2021 HTTP client were also pulling in WebSocket libraries, as the
server side always needs to speak websockets, but only GOOS=js clients
speak it.

This doesn't yet totally remove the websocket dependency on Linux because
Linux has a envknob opt-in to act like GOOS=js for manual testing and force
the use of WebSockets for DERP only (not control). We can put that behind
a build tag in a future change to eliminate the dep on all GOOSes.

Updates tailscale#1278

Change-Id: I4f60508f4cad52bf8c8943c8851ecee506b7ebc9
Signed-off-by: Brad Fitzpatrick <[email protected]>

Author: bradfitz

cmd/k8s-operator/depaware.txt

@@ -80,10 +80,10 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/
         github.com/beorn7/perks/quantile                             from github.com/prometheus/client_golang/prometheus
         github.com/bits-and-blooms/bitset                            from github.com/gaissmai/bart
      💣 github.com/cespare/xxhash/v2                                 from github.com/prometheus/client_golang/prometheus
-        github.com/coder/websocket                                   from tailscale.com/control/controlhttp+
-        github.com/coder/websocket/internal/errd                     from github.com/coder/websocket
-        github.com/coder/websocket/internal/util                     from github.com/coder/websocket
-        github.com/coder/websocket/internal/xsync                    from github.com/coder/websocket
+   L    github.com/coder/websocket                                   from tailscale.com/derp/derphttp+
+   L    github.com/coder/websocket/internal/errd                     from github.com/coder/websocket
+   L    github.com/coder/websocket/internal/util                     from github.com/coder/websocket
+   L    github.com/coder/websocket/internal/xsync                    from github.com/coder/websocket
    L    github.com/coreos/go-iptables/iptables                       from tailscale.com/util/linuxfw
      💣 github.com/davecgh/go-spew/spew                              from k8s.io/apimachinery/pkg/util/dump
    W 💣 github.com/dblohm7/wingoes                                   from github.com/dblohm7/wingoes/com+
@@ -658,6 +658,7 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/
         tailscale.com/control/controlbase                            from tailscale.com/control/controlhttp+
         tailscale.com/control/controlclient                          from tailscale.com/ipn/ipnlocal+
         tailscale.com/control/controlhttp                            from tailscale.com/control/controlclient
+        tailscale.com/control/controlhttp/controlhttpcommon          from tailscale.com/control/controlhttp
         tailscale.com/control/controlknobs                           from tailscale.com/control/controlclient+
         tailscale.com/derp                                           from tailscale.com/derp/derphttp+
         tailscale.com/derp/derphttp                                  from tailscale.com/ipn/localapi+
@@ -740,7 +741,7 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/
         tailscale.com/net/tsdial                                     from tailscale.com/control/controlclient+
      💣 tailscale.com/net/tshttpproxy                                from tailscale.com/clientupdate/distsign+
         tailscale.com/net/tstun                                      from tailscale.com/tsd+
-        tailscale.com/net/wsconn                                     from tailscale.com/control/controlhttp+
+   L    tailscale.com/net/wsconn                                     from tailscale.com/derp/derphttp
         tailscale.com/omit                                           from tailscale.com/ipn/conffile
         tailscale.com/paths                                          from tailscale.com/client/tailscale+
      💣 tailscale.com/portlist                                       from tailscale.com/ipn/ipnlocal

cmd/tailscale/depaware.txt

@@ -5,10 +5,10 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep
    W 💣 github.com/alexbrainman/sspi                                 from github.com/alexbrainman/sspi/internal/common+
    W    github.com/alexbrainman/sspi/internal/common                 from github.com/alexbrainman/sspi/negotiate
    W 💣 github.com/alexbrainman/sspi/negotiate                       from tailscale.com/net/tshttpproxy
-        github.com/coder/websocket                                   from tailscale.com/control/controlhttp+
-        github.com/coder/websocket/internal/errd                     from github.com/coder/websocket
-        github.com/coder/websocket/internal/util                     from github.com/coder/websocket
-        github.com/coder/websocket/internal/xsync                    from github.com/coder/websocket
+   L    github.com/coder/websocket                                   from tailscale.com/derp/derphttp+
+   L    github.com/coder/websocket/internal/errd                     from github.com/coder/websocket
+   L    github.com/coder/websocket/internal/util                     from github.com/coder/websocket
+   L    github.com/coder/websocket/internal/xsync                    from github.com/coder/websocket
    L    github.com/coreos/go-iptables/iptables                       from tailscale.com/util/linuxfw
    W 💣 github.com/dblohm7/wingoes                                   from github.com/dblohm7/wingoes/pe+
    W 💣 github.com/dblohm7/wingoes/pe                                from tailscale.com/util/winutil/authenticode
@@ -86,6 +86,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep
         tailscale.com/cmd/tailscale/cli/ffcomplete/internal          from tailscale.com/cmd/tailscale/cli/ffcomplete
         tailscale.com/control/controlbase                            from tailscale.com/control/controlhttp+
         tailscale.com/control/controlhttp                            from tailscale.com/cmd/tailscale/cli
+        tailscale.com/control/controlhttp/controlhttpcommon          from tailscale.com/control/controlhttp
         tailscale.com/control/controlknobs                           from tailscale.com/net/portmapper
         tailscale.com/derp                                           from tailscale.com/derp/derphttp
         tailscale.com/derp/derphttp                                  from tailscale.com/net/netcheck
@@ -124,7 +125,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep
         tailscale.com/net/tlsdial/blockblame                         from tailscale.com/net/tlsdial
         tailscale.com/net/tsaddr                                     from tailscale.com/client/web+
      💣 tailscale.com/net/tshttpproxy                                from tailscale.com/clientupdate/distsign+
-        tailscale.com/net/wsconn                                     from tailscale.com/control/controlhttp+
+   L    tailscale.com/net/wsconn                                     from tailscale.com/derp/derphttp
         tailscale.com/paths                                          from tailscale.com/client/tailscale+
      💣 tailscale.com/safesocket                                     from tailscale.com/client/tailscale+
         tailscale.com/syncs                                          from tailscale.com/cmd/tailscale/cli+

cmd/tailscaled/depaware.txt

@@ -79,10 +79,10 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
    L    github.com/aws/smithy-go/transport/http/internal/io          from github.com/aws/smithy-go/transport/http
    L    github.com/aws/smithy-go/waiter                              from github.com/aws/aws-sdk-go-v2/service/ssm
         github.com/bits-and-blooms/bitset                            from github.com/gaissmai/bart
-        github.com/coder/websocket                                   from tailscale.com/control/controlhttp+
-        github.com/coder/websocket/internal/errd                     from github.com/coder/websocket
-        github.com/coder/websocket/internal/util                     from github.com/coder/websocket
-        github.com/coder/websocket/internal/xsync                    from github.com/coder/websocket
+   L    github.com/coder/websocket                                   from tailscale.com/derp/derphttp+
+   L    github.com/coder/websocket/internal/errd                     from github.com/coder/websocket
+   L    github.com/coder/websocket/internal/util                     from github.com/coder/websocket
+   L    github.com/coder/websocket/internal/xsync                    from github.com/coder/websocket
    L    github.com/coreos/go-iptables/iptables                       from tailscale.com/util/linuxfw
   LD 💣 github.com/creack/pty                                        from tailscale.com/ssh/tailssh
    W 💣 github.com/dblohm7/wingoes                                   from github.com/dblohm7/wingoes/com+
@@ -249,6 +249,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/control/controlbase                            from tailscale.com/control/controlhttp+
         tailscale.com/control/controlclient                          from tailscale.com/cmd/tailscaled+
         tailscale.com/control/controlhttp                            from tailscale.com/control/controlclient
+        tailscale.com/control/controlhttp/controlhttpcommon          from tailscale.com/control/controlhttp
         tailscale.com/control/controlknobs                           from tailscale.com/control/controlclient+
         tailscale.com/derp                                           from tailscale.com/derp/derphttp+
         tailscale.com/derp/derphttp                                  from tailscale.com/cmd/tailscaled+
@@ -327,7 +328,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/net/tsdial                                     from tailscale.com/cmd/tailscaled+
      💣 tailscale.com/net/tshttpproxy                                from tailscale.com/clientupdate/distsign+
         tailscale.com/net/tstun                                      from tailscale.com/cmd/tailscaled+
-        tailscale.com/net/wsconn                                     from tailscale.com/control/controlhttp+
+   L    tailscale.com/net/wsconn                                     from tailscale.com/derp/derphttp
         tailscale.com/omit                                           from tailscale.com/ipn/conffile
         tailscale.com/paths                                          from tailscale.com/client/tailscale+
      💣 tailscale.com/portlist                                       from tailscale.com/ipn/ipnlocal

control/controlclient/noise_test.go

@@ -15,7 +15,7 @@ import (
 	"time"
 
 	"golang.org/x/net/http2"
-	"tailscale.com/control/controlhttp"
+	"tailscale.com/control/controlhttp/controlhttpserver"
 	"tailscale.com/internal/noiseconn"
 	"tailscale.com/net/netmon"
 	"tailscale.com/net/tsdial"
@@ -201,7 +201,7 @@ func (up *Upgrader) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return nil
 	}
 
-	cbConn, err := controlhttp.AcceptHTTP(r.Context(), w, r, up.noiseKeyPriv, earlyWriteFn)
+	cbConn, err := controlhttpserver.AcceptHTTP(r.Context(), w, r, up.noiseKeyPriv, earlyWriteFn)
 	if err != nil {
 		up.logf("controlhttp: Accept: %v", err)
 		return

control/controlhttp/client.go

@@ -38,6 +38,7 @@ import (
 	"time"
 
 	"tailscale.com/control/controlbase"
+	"tailscale.com/control/controlhttp/controlhttpcommon"
 	"tailscale.com/envknob"
 	"tailscale.com/health"
 	"tailscale.com/net/dnscache"
@@ -571,9 +572,9 @@ func (a *Dialer) tryURLUpgrade(ctx context.Context, u *url.URL, optAddr netip.Ad
 		Method: "POST",
 		URL:    u,
 		Header: http.Header{
-			"Upgrade":           []string{upgradeHeaderValue},
-			"Connection":        []string{"upgrade"},
-			handshakeHeaderName: []string{base64.StdEncoding.EncodeToString(init)},
+			"Upgrade":                             []string{controlhttpcommon.UpgradeHeaderValue},
+			"Connection":                          []string{"upgrade"},
+			controlhttpcommon.HandshakeHeaderName: []string{base64.StdEncoding.EncodeToString(init)},
 		},
 	}
 	req = req.WithContext(ctx)
@@ -597,7 +598,7 @@ func (a *Dialer) tryURLUpgrade(ctx context.Context, u *url.URL, optAddr netip.Ad
 		return nil, fmt.Errorf("httptrace didn't provide a connection")
 	}
 
-	if next := resp.Header.Get("Upgrade"); next != upgradeHeaderValue {
+	if next := resp.Header.Get("Upgrade"); next != controlhttpcommon.UpgradeHeaderValue {
 		resp.Body.Close()
 		return nil, fmt.Errorf("server switched to unexpected protocol %q", next)
 	}

control/controlhttp/constants.go

@@ -18,15 +18,6 @@ import (
 )
 
 const (
-	// upgradeHeader is the value of the Upgrade HTTP header used to
-	// indicate the Tailscale control protocol.
-	upgradeHeaderValue = "tailscale-control-protocol"
-
-	// handshakeHeaderName is the HTTP request header that can
-	// optionally contain base64-encoded initial handshake
-	// payload, to save an RTT.
-	handshakeHeaderName = "X-Tailscale-Handshake"
-
 	// serverUpgradePath is where the server-side HTTP handler to
 	// to do the protocol switch is located.
 	serverUpgradePath = "/ts2021"

control/controlhttp/controlhttpcommon/controlhttpcommon.go

@@ -0,0 +1,15 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+// Package controlhttpcommon contains common constants for used
+// by the controlhttp client and controlhttpserver packages.
+package controlhttpcommon
+
+// UpgradeHeader is the value of the Upgrade HTTP header used to
+// indicate the Tailscale control protocol.
+const UpgradeHeaderValue = "tailscale-control-protocol"
+
+// handshakeHeaderName is the HTTP request header that can
+// optionally contain base64-encoded initial handshake
+// payload, to save an RTT.
+const HandshakeHeaderName = "X-Tailscale-Handshake"

control/controlhttp/server.go renamed to control/controlhttp/controlhttpserver/controlhttpserver.go

@@ -3,7 +3,8 @@
 
 //go:build !ios
 
-package controlhttp
+// Packet controlhttpserver contains the HTTP server side of the ts2021 control protocol.
+package controlhttpserver
 
 import (
 	"context"
@@ -18,6 +19,7 @@ import (
 
 	"github.com/coder/websocket"
 	"tailscale.com/control/controlbase"
+	"tailscale.com/control/controlhttp/controlhttpcommon"
 	"tailscale.com/net/netutil"
 	"tailscale.com/net/wsconn"
 	"tailscale.com/types/key"
@@ -45,12 +47,12 @@ func acceptHTTP(ctx context.Context, w http.ResponseWriter, r *http.Request, pri
 	if next == "websocket" {
 		return acceptWebsocket(ctx, w, r, private)
 	}
-	if next != upgradeHeaderValue {
+	if next != controlhttpcommon.UpgradeHeaderValue {
 		http.Error(w, "unknown next protocol", http.StatusBadRequest)
 		return nil, fmt.Errorf("client requested unhandled next protocol %q", next)
 	}
 
-	initB64 := r.Header.Get(handshakeHeaderName)
+	initB64 := r.Header.Get(controlhttpcommon.HandshakeHeaderName)
 	if initB64 == "" {
 		http.Error(w, "missing Tailscale handshake header", http.StatusBadRequest)
 		return nil, errors.New("no tailscale handshake header in HTTP request")
@@ -67,7 +69,7 @@ func acceptHTTP(ctx context.Context, w http.ResponseWriter, r *http.Request, pri
 		return nil, errors.New("can't hijack client connection")
 	}
 
-	w.Header().Set("Upgrade", upgradeHeaderValue)
+	w.Header().Set("Upgrade", controlhttpcommon.UpgradeHeaderValue)
 	w.Header().Set("Connection", "upgrade")
 	w.WriteHeader(http.StatusSwitchingProtocols)
 
@@ -117,7 +119,7 @@ func acceptHTTP(ctx context.Context, w http.ResponseWriter, r *http.Request, pri
 // speak HTTP) to a Tailscale control protocol base transport connection.
 func acceptWebsocket(ctx context.Context, w http.ResponseWriter, r *http.Request, private key.MachinePrivate) (*controlbase.Conn, error) {
 	c, err := websocket.Accept(w, r, &websocket.AcceptOptions{
-		Subprotocols:   []string{upgradeHeaderValue},
+		Subprotocols:   []string{controlhttpcommon.UpgradeHeaderValue},
 		OriginPatterns: []string{"*"},
 		// Disable compression because we transmit Noise messages that are not
 		// compressible.
@@ -129,15 +131,15 @@ func acceptWebsocket(ctx context.Context, w http.ResponseWriter, r *http.Request
 	if err != nil {
 		return nil, fmt.Errorf("Could not accept WebSocket connection %v", err)
 	}
-	if c.Subprotocol() != upgradeHeaderValue {
+	if c.Subprotocol() != controlhttpcommon.UpgradeHeaderValue {
 		c.Close(websocket.StatusPolicyViolation, "client must speak the control subprotocol")
 		return nil, fmt.Errorf("Unexpected subprotocol %q", c.Subprotocol())
 	}
 	if err := r.ParseForm(); err != nil {
 		c.Close(websocket.StatusPolicyViolation, "Could not parse parameters")
 		return nil, fmt.Errorf("parse query parameters: %v", err)
 	}
-	initB64 := r.Form.Get(handshakeHeaderName)
+	initB64 := r.Form.Get(controlhttpcommon.HandshakeHeaderName)
 	if initB64 == "" {
 		c.Close(websocket.StatusPolicyViolation, "missing Tailscale handshake parameter")
 		return nil, errors.New("no tailscale handshake parameter in HTTP request")

control/controlhttp/http_test.go

@@ -23,12 +23,15 @@ import (
 	"time"
 
 	"tailscale.com/control/controlbase"
+	"tailscale.com/control/controlhttp/controlhttpcommon"
+	"tailscale.com/control/controlhttp/controlhttpserver"
 	"tailscale.com/net/dnscache"
 	"tailscale.com/net/netmon"
 	"tailscale.com/net/socks5"
 	"tailscale.com/net/tsdial"
 	"tailscale.com/tailcfg"
 	"tailscale.com/tstest"
+	"tailscale.com/tstest/deptest"
 	"tailscale.com/tstime"
 	"tailscale.com/types/key"
 	"tailscale.com/types/logger"
@@ -158,7 +161,7 @@ func testControlHTTP(t *testing.T, param httpTestParam) {
 				return err
 			}
 		}
-		conn, err := AcceptHTTP(context.Background(), w, r, server, earlyWriteFn)
+		conn, err := controlhttpserver.AcceptHTTP(context.Background(), w, r, server, earlyWriteFn)
 		if err != nil {
 			log.Print(err)
 		}
@@ -529,7 +532,7 @@ EKTcWGekdmdDPsHloRNtsiCa697B2O9IFA==
 
 func brokenMITMHandler(clock tstime.Clock) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
-		w.Header().Set("Upgrade", upgradeHeaderValue)
+		w.Header().Set("Upgrade", controlhttpcommon.UpgradeHeaderValue)
 		w.Header().Set("Connection", "upgrade")
 		w.WriteHeader(http.StatusSwitchingProtocols)
 		w.(http.Flusher).Flush()
@@ -574,7 +577,7 @@ func TestDialPlan(t *testing.T) {
 			close(done)
 		})
 		var handler http.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			conn, err := AcceptHTTP(context.Background(), w, r, server, nil)
+			conn, err := controlhttpserver.AcceptHTTP(context.Background(), w, r, server, nil)
 			if err != nil {
 				log.Print(err)
 			} else {
@@ -816,3 +819,14 @@ func (c *closeTrackConn) Close() error {
 	c.d.noteClose(c)
 	return c.Conn.Close()
 }
+
+func TestDeps(t *testing.T) {
+	deptest.DepChecker{
+		GOOS:   "darwin",
+		GOARCH: "arm64",
+		BadDeps: map[string]string{
+			// Only the controlhttpserver needs WebSockets...
+			"github.com/coder/websocket": "controlhttp client shouldn't need websockets",
+		},
+	}.Check(t)
+}

tstest/integration/testcontrol/testcontrol.go

@@ -26,7 +26,7 @@ import (
 	"time"
 
 	"golang.org/x/net/http2"
-	"tailscale.com/control/controlhttp"
+	"tailscale.com/control/controlhttp/controlhttpserver"
 	"tailscale.com/net/netaddr"
 	"tailscale.com/net/tsaddr"
 	"tailscale.com/tailcfg"
@@ -288,7 +288,7 @@ func (s *Server) serveNoiseUpgrade(w http.ResponseWriter, r *http.Request) {
 	s.mu.Lock()
 	noisePrivate := s.noisePrivKey
 	s.mu.Unlock()
-	cc, err := controlhttp.AcceptHTTP(ctx, w, r, noisePrivate, nil)
+	cc, err := controlhttpserver.AcceptHTTP(ctx, w, r, noisePrivate, nil)
 	if err != nil {
 		log.Printf("AcceptHTTP: %v", err)
 		return