From dfe67afb4a19af889fe0ecf4e8b64ca3cb897b39 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Fri, 28 Oct 2022 12:14:58 -0700 Subject: [PATCH] control/controlhttp: remove ClientConn.UntrustedUpgradeHeaders It was just added and unreleased but we've decided to go a different route. Details are in 5e9e57ecf531f. Updates #5972 Change-Id: I49016af469225f58535f63a9b0fbe5ab6a5bf304 Signed-off-by: Brad Fitzpatrick --- control/controlhttp/client.go | 19 +++++++++---------- control/controlhttp/client_common.go | 8 -------- control/controlhttp/client_js.go | 7 ++----- 3 files changed, 11 insertions(+), 23 deletions(-) diff --git a/control/controlhttp/client.go b/control/controlhttp/client.go index b27ef6279..4bf8b9b6b 100644 --- a/control/controlhttp/client.go +++ b/control/controlhttp/client.go @@ -336,7 +336,7 @@ func (a *Dialer) dialURL(ctx context.Context, u *url.URL, addr netip.Addr) (*Cli if err != nil { return nil, err } - netConn, untrustedUpgradeHeaders, err := a.tryURLUpgrade(ctx, u, addr, init) + netConn, err := a.tryURLUpgrade(ctx, u, addr, init) if err != nil { return nil, err } @@ -346,8 +346,7 @@ func (a *Dialer) dialURL(ctx context.Context, u *url.URL, addr netip.Addr) (*Cli return nil, err } return &ClientConn{ - Conn: cbConn, - UntrustedUpgradeHeaders: untrustedUpgradeHeaders, + Conn: cbConn, }, nil } @@ -356,7 +355,7 @@ func (a *Dialer) dialURL(ctx context.Context, u *url.URL, addr netip.Addr) (*Cli // provided address. // // Only the provided ctx is used, not a.ctx. -func (a *Dialer) tryURLUpgrade(ctx context.Context, u *url.URL, addr netip.Addr, init []byte) (_ net.Conn, untrustedUpgradeHeaders http.Header, _ error) { +func (a *Dialer) tryURLUpgrade(ctx context.Context, u *url.URL, addr netip.Addr, init []byte) (net.Conn, error) { var dns *dnscache.Resolver // If we were provided an address to dial, then create a resolver that just @@ -438,11 +437,11 @@ func (a *Dialer) tryURLUpgrade(ctx context.Context, u *url.URL, addr netip.Addr, resp, err := tr.RoundTrip(req) if err != nil { - return nil, nil, err + return nil, err } if resp.StatusCode != http.StatusSwitchingProtocols { - return nil, nil, fmt.Errorf("unexpected HTTP response: %s", resp.Status) + return nil, fmt.Errorf("unexpected HTTP response: %s", resp.Status) } // From here on, the underlying net.Conn is ours to use, but there @@ -456,19 +455,19 @@ func (a *Dialer) tryURLUpgrade(ctx context.Context, u *url.URL, addr netip.Addr, } if switchedConn == nil { resp.Body.Close() - return nil, nil, fmt.Errorf("httptrace didn't provide a connection") + return nil, fmt.Errorf("httptrace didn't provide a connection") } if next := resp.Header.Get("Upgrade"); next != upgradeHeaderValue { resp.Body.Close() - return nil, nil, fmt.Errorf("server switched to unexpected protocol %q", next) + return nil, fmt.Errorf("server switched to unexpected protocol %q", next) } rwc, ok := resp.Body.(io.ReadWriteCloser) if !ok { resp.Body.Close() - return nil, nil, errors.New("http Transport did not provide a writable body") + return nil, errors.New("http Transport did not provide a writable body") } - return netutil.NewAltReadWriteCloserConn(rwc, switchedConn), resp.Header, nil + return netutil.NewAltReadWriteCloserConn(rwc, switchedConn), nil } diff --git a/control/controlhttp/client_common.go b/control/controlhttp/client_common.go index dfc32b639..b8b28132f 100644 --- a/control/controlhttp/client_common.go +++ b/control/controlhttp/client_common.go @@ -5,8 +5,6 @@ package controlhttp import ( - "net/http" - "tailscale.com/control/controlbase" ) @@ -17,10 +15,4 @@ import ( type ClientConn struct { // Conn is the noise connection. *controlbase.Conn - - // UntrustedUpgradeHeaders are the HTTP headers seen in the - // 101 Switching Protocols upgrade response. They may be nil - // or even might've been tampered with by a middlebox. - // They should not be trusted. - UntrustedUpgradeHeaders http.Header } diff --git a/control/controlhttp/client_js.go b/control/controlhttp/client_js.go index feb660cfa..0dfb82b91 100644 --- a/control/controlhttp/client_js.go +++ b/control/controlhttp/client_js.go @@ -46,7 +46,7 @@ func (d *Dialer) Dial(ctx context.Context) (*ClientConn, error) { handshakeHeaderName: []string{base64.StdEncoding.EncodeToString(init)}, }.Encode(), } - wsConn, httpRes, err := websocket.Dial(ctx, wsURL.String(), &websocket.DialOptions{ + wsConn, _, err := websocket.Dial(ctx, wsURL.String(), &websocket.DialOptions{ Subprotocols: []string{upgradeHeaderValue}, }) if err != nil { @@ -58,8 +58,5 @@ func (d *Dialer) Dial(ctx context.Context) (*ClientConn, error) { netConn.Close() return nil, err } - return &ClientConn{ - Conn: cbConn, - UntrustedUpgradeHeaders: httpRes.Header, - }, nil + return &ClientConn{Conn: cbConn}, nil }