ipn/ipnlocal: use h2c for all HTTP/2 requests to plaintext backendsrajsinghtech/serve-h2c-all-http2

The serve reverse proxy previously only used h2c transport for requests
with gRPC content types (application/grpc). This blocked other HTTP/2
protocols like Connect RPC from working through tailscale serve and
ListenService, since their content types (application/connect+proto,
application/connect+json) didn't match the gRPC sniff.

Remove the content-type gate and use h2c for any HTTP/2 request to a
plaintext (http:// or unix socket) backend. The ProtoMajor == 2 check
is the correct semantic boundary — if the frontend negotiated HTTP/2,
the plaintext backend is expected to speak h2c.

Updates #18483

Signed-off-by: Raj Singh <raj@tailscale.com>

2 files changed, 39 insertions, 57 deletions

diff --git a/ipn/ipnlocal/serve.go b/ipn/ipnlocal/serve.go
index d25251acc..937b82d85 100644
--- a/ipn/ipnlocal/serve.go
+++ b/ipn/ipnlocal/serve.go
@@ -67,11 +67,6 @@ func init() {
 	RegisterC2N("GET /vip-services", handleC2NVIPServicesGet)
 }
 
-const (
-	contentTypeHeader   = "Content-Type"
-	grpcBaseContentType = "application/grpc"
-)
-
 // ErrETagMismatch signals that the given
 // If-Match header does not match with the
 // current etag of a resource.
@@ -887,8 +882,8 @@ func (b *LocalBackend) isTailscaledSocket(socketPath string) bool {
 // reverseProxy is a proxy that forwards a request to a backend host
 // (preconfigured via ipn.ServeConfig). If the host is configured with
 // http+insecure prefix, connection between proxy and backend will be over
-// insecure TLS. If the backend host has a http prefix and the incoming request
-// has application/grpc content type header, the connection will be over h2c.
+// insecure TLS. If the backend is plaintext (http:// or unix socket) and the
+// incoming request is HTTP/2, the connection will be over h2c.
 // Otherwise standard Go http transport will be used.
 type reverseProxy struct {
 	logf logger.Logf
@@ -951,13 +946,10 @@ func (rp *reverseProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 			http.Error(w, err.Error(), http.StatusInternalServerError)
 			return
 		}
-	}} // There is no way to autodetect h2c as per RFC 9113
-	// https://datatracker.ietf.org/doc/html/rfc9113#name-starting-http-2.
-	// However, we assume that http:// proxy prefix in combination with the
-	// protoccol being HTTP/2 is sufficient to detect h2c for our needs. Only use this for
-	// gRPC to fix a known problem of plaintext gRPC backends
+	}} // Use h2c transport for HTTP/2 requests to plaintext backends. This is
+	// needed for protocols that require HTTP/2 (gRPC, Connect RPC, etc.)
+	// when the backend is running without TLS.
 	if rp.shouldProxyViaH2C(r) {
-		rp.logf("received a proxy request for plaintext gRPC")
 		p.Transport = rp.getH2CTransport()
 	} else {
 		p.Transport = rp.getTransport()
@@ -965,7 +957,7 @@ func (rp *reverseProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	p.ServeHTTP(w, r)
 }
 
-// getTransport returns the Transport used for regular (non-GRPC) requests
+// getTransport returns the Transport used for regular (non-h2c) requests
 // to the backend. The Transport gets created lazily, at most once.
 func (rp *reverseProxy) getTransport() *http.Transport {
 	return rp.httpTransport.Get(func() *http.Transport {
@@ -992,8 +984,8 @@ func (rp *reverseProxy) getTransport() *http.Transport {
 	})
 }
 
-// getH2CTransport returns the Transport used for GRPC requests to the backend.
-// The Transport gets created lazily, at most once.
+// getH2CTransport returns the Transport used for HTTP/2 cleartext (h2c)
+// requests to the backend. The Transport gets created lazily, at most once.
 func (rp *reverseProxy) getH2CTransport() http.RoundTripper {
 	return rp.h2cTransport.Get(func() *http.Transport {
 		var p http.Protocols
@@ -1012,25 +1004,18 @@ func (rp *reverseProxy) getH2CTransport() http.RoundTripper {
 	})
 }
 
-// This is not a generally reliable way how to determine whether a request is
-// for a h2c server, but sufficient for our particular use case.
+// shouldProxyViaH2C reports whether the request should be proxied using h2c
+// (HTTP/2 over cleartext TCP). This is used when the inbound connection is
+// HTTP/2 and the backend is a plaintext (http:// or unix socket) target,
+// supporting any protocol that requires HTTP/2 (gRPC, Connect RPC, etc.).
 func (rp *reverseProxy) shouldProxyViaH2C(r *http.Request) bool {
-	contentType := r.Header.Get(contentTypeHeader)
-	// For unix sockets, check if it's gRPC content to determine h2c
+	if r.ProtoMajor != 2 {
+		return false
+	}
 	if rp.socketPath != "" {
-		return r.ProtoMajor == 2 && isGRPCContentType(contentType)
+		return true
 	}
-	return r.ProtoMajor == 2 && strings.HasPrefix(rp.backend, "http://") && isGRPCContentType(contentType)
-}
-
-// isGRPC accepts an HTTP request's content type header value and determines
-// whether this is gRPC content. grpc-go considers a value that equals
-// application/grpc or has a prefix of application/grpc+ or application/grpc; a
-// valid grpc content type header.
-// https://github.com/grpc/grpc-go/blob/v1.60.0-dev/internal/grpcutil/method.go#L41-L78
-func isGRPCContentType(contentType string) bool {
-	s, ok := strings.CutPrefix(contentType, grpcBaseContentType)
-	return ok && (len(s) == 0 || s[0] == '+' || s[0] == ';')
+	return strings.HasPrefix(rp.backend, "http://")
 }
 
 func addProxyForwardedHeaders(r *httputil.ProxyRequest) {
diff --git a/ipn/ipnlocal/serve_test.go b/ipn/ipnlocal/serve_test.go
index b3f48b105..b541933f7 100644
--- a/ipn/ipnlocal/serve_test.go
+++ b/ipn/ipnlocal/serve_test.go
@@ -1235,26 +1235,6 @@ func TestServeFileOrDirectory(t *testing.T) {
 	}
 }
 
-func Test_isGRPCContentType(t *testing.T) {
-	tests := []struct {
-		contentType string
-		want        bool
-	}{
-		{contentType: "application/grpc", want: true},
-		{contentType: "application/grpc;", want: true},
-		{contentType: "application/grpc+", want: true},
-		{contentType: "application/grpcfoobar"},
-		{contentType: "application/text"},
-		{contentType: "foobar"},
-		{contentType: ""},
-	}
-	for _, tt := range tests {
-		if got := isGRPCContentType(tt.contentType); got != tt.want {
-			t.Errorf("isGRPCContentType(%q) = %v, want %v", tt.contentType, got, tt.want)
-		}
-	}
-}
-
 func TestEncTailscaleHeaderValue(t *testing.T) {
 	tests := []struct {
 		in   string
@@ -1273,7 +1253,7 @@ func TestEncTailscaleHeaderValue(t *testing.T) {
 	}
 }
 
-func TestServeGRPCProxy(t *testing.T) {
+func TestServeH2CProxy(t *testing.T) {
 	const msg = "some-response\n"
 	backend := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Path-Was", r.RequestURI)
@@ -1317,26 +1297,43 @@ func TestServeGRPCProxy(t *testing.T) {
 		req       *http.Request
 		wantPath  string
 		wantProto string
-		wantBody  string
 	}{
 		{
-			name:      "non-gRPC",
+			name:      "http1-no-h2c",
 			req:       req("GET", "http://foo/bar"),
 			wantPath:  "/bar",
 			wantProto: "HTTP/1.1",
 		},
 		{
-			name:      "gRPC-but-not-http2",
+			name:      "http1-with-grpc-content-type",
 			req:       req("GET", "http://foo/bar", "application/grpc"),
 			wantPath:  "/bar",
 			wantProto: "HTTP/1.1",
 		},
 		{
-			name:      "gRPC--http2",
+			name:      "http2-grpc",
 			req:       req("GET", "http://foo/bar", 2, "application/grpc"),
 			wantPath:  "/bar",
 			wantProto: "HTTP/2.0",
 		},
+		{
+			name:      "http2-connect-rpc-proto",
+			req:       req("POST", "http://foo/bar", 2, "application/connect+proto"),
+			wantPath:  "/bar",
+			wantProto: "HTTP/2.0",
+		},
+		{
+			name:      "http2-connect-rpc-json",
+			req:       req("POST", "http://foo/bar", 2, "application/connect+json"),
+			wantPath:  "/bar",
+			wantProto: "HTTP/2.0",
+		},
+		{
+			name:      "http2-no-content-type",
+			req:       req("GET", "http://foo/bar", 2),
+			wantPath:  "/bar",
+			wantProto: "HTTP/2.0",
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {