fix: force websockets when http2 is negotiated (#16)

kylecarbs · web-flow · commit 058fa46a3723 · 2023-03-27T15:54:51.000-05:00
diff --git a/derp/derphttp/derphttp_client.go b/derp/derphttp/derphttp_client.go
@@ -477,6 +477,16 @@ func (c *Client) connect(ctx context.Context, caller string) (client *derp.Clien
 	req.Header.Set("Upgrade", "DERP")
 	req.Header.Set("Connection", "Upgrade")
 
+	regionID := 0
+	if reg != nil {
+		regionID = reg.RegionID
+	}
+	if tlsState != nil && tlsState.NegotiatedProtocol == "h2" {
+		reason := "The server wanted us to use HTTP/2, but DERP requires Upgrade which needs HTTP/1.1"
+		c.forceWebsockets(regionID, reason)
+		return nil, 0, fmt.Errorf("DERP server did not switch protocols: %s", reason)
+	}
+
 	if !serverPub.IsZero() && serverProtoVersion != 0 {
 		// parseMetaCert found the server's public key (no TLS
 		// middlebox was in the way), so skip the HTTP upgrade
@@ -492,17 +502,6 @@ func (c *Client) connect(ctx context.Context, caller string) (client *derp.Clien
 		// No need to flush the HTTP request. the derp.Client's initial
 		// client auth frame will flush it.
 	} else {
-		regionID := 0
-		if reg != nil {
-			regionID = reg.RegionID
-		}
-
-		if tlsState != nil && tlsState.NegotiatedProtocol == "h2" {
-			reason := fmt.Sprintf("The server wanted us to use HTTP/2, but DERP requires Upgrade which needs HTTP/1.1")
-			c.forceWebsockets(regionID, reason)
-			return nil, 0, fmt.Errorf("DERP server did not switch protocols: %s", reason)
-		}
-
 		if err := req.Write(brw); err != nil {
 			return nil, 0, err
 		}
@@ -650,7 +649,7 @@ func (c *Client) tlsConfig(node *tailcfg.DERPNode) *tls.Config {
 			tlsdial.SetConfigExpectedCert(tlsConf, node.CertName)
 		}
 	}
-	tlsConf.NextProtos = []string{"h2", "http/1.1"}
+	tlsConf.NextProtos = []string{"http/1.1", "h2"}
 	return tlsConf
 }
 
diff --git a/derp/derphttp/derphttp_test.go b/derp/derphttp/derphttp_test.go
@@ -235,7 +235,13 @@ func TestHTTP2WebSocketFallback(t *testing.T) {
 		s.Accept(context.Background(), wc, brw, r.RemoteAddr)
 	}))
 	httpsrv.TLS = &tls.Config{
-		NextProtos: []string{"h2", "http/1.1"},
+		NextProtos: []string{"h2"},
+		GetCertificate: func(chi *tls.ClientHelloInfo) (*tls.Certificate, error) {
+			// Add this to ensure fast start works!
+			cert := httpsrv.TLS.Certificates[0]
+			cert.Certificate = append(cert.Certificate, s.MetaCert())
+			return &cert, nil
+		},
 	}
 	httpsrv.StartTLS()
 
@@ -264,7 +270,7 @@ func TestHTTP2WebSocketFallback(t *testing.T) {
 		t.Fatal("client didn't error on initial connect")
 	}
 	reason := <-reasonCh
-	if !strings.Contains(reason, "wanted us to use HTTP/2") {
+	if !strings.Contains(reason, "DERP requires Upgrade which needs") {
 		t.Fatalf("reason doesn't contain message: %s", reason)
 	}
 	if err := c.Connect(context.Background()); err != nil {
