From 26a35253375af9fade678862834288fff4a6ad79 Mon Sep 17 00:00:00 2001
From: Marten Seemann <martenseemann@gmail.com>
Date: Sun, 1 Sep 2024 17:20:35 +0800
Subject: [PATCH] http3: reject connection-specific header fields, check value
 of TE (#4655)

---
 http3/headers.go                   | 19 +++++++++++++++++--
 http3/headers_test.go              | 29 +++++++++++++++++++++++++----
 integrationtests/self/http_test.go |  6 ++----
 3 files changed, 44 insertions(+), 10 deletions(-)

diff --git a/http3/headers.go b/http3/headers.go
index 3f17f222..a637149a 100644
--- a/http3/headers.go
+++ b/http3/headers.go
@@ -28,6 +28,15 @@ type header struct {
 	Headers http.Header
 }
 
+// connection-specific header fields must not be sent on HTTP/3
+var invalidHeaderFields = [...]string{
+	"connection",
+	"keep-alive",
+	"proxy-connection",
+	"transfer-encoding",
+	"upgrade",
+}
+
 func parseHeaders(headers []qpack.HeaderField, isRequest bool) (header, error) {
 	hdr := header{Headers: make(http.Header, len(headers))}
 	var readFirstRegularHeader, readContentLength bool
@@ -73,10 +82,16 @@ func parseHeaders(headers []qpack.HeaderField, isRequest bool) (header, error) {
 			if !httpguts.ValidHeaderFieldName(h.Name) {
 				return header{}, fmt.Errorf("invalid header field name: %q", h.Name)
 			}
+			for _, invalidField := range invalidHeaderFields {
+				if h.Name == invalidField {
+					return header{}, fmt.Errorf("invalid header field name: %q", h.Name)
+				}
+			}
+			if h.Name == "te" && h.Value != "trailers" {
+				return header{}, fmt.Errorf("invalid TE header field value: %q", h.Value)
+			}
 			readFirstRegularHeader = true
 			switch h.Name {
-			case "transfer-encoding":
-				return header{}, errors.New("invalid header field: Transfer-Encoding")
 			case "content-length":
 				// Ignore duplicate Content-Length headers.
 				// Fail if the duplicates differ.
diff --git a/http3/headers_test.go b/http3/headers_test.go
index 5bc41e31..c5396f0d 100644
--- a/http3/headers_test.go
+++ b/http3/headers_test.go
@@ -1,6 +1,7 @@
 package http3
 
 import (
+	"fmt"
 	"net/http"
 	"net/url"
 
@@ -369,13 +370,33 @@ var _ = Describe("Response", func() {
 		Expect(err).To(MatchError("invalid response pseudo header: :method"))
 	})
 
-	It("rejects the Transfer-Encoding header field", func() {
+	DescribeTable("rejecting invalid header fields",
+		func(invalidField string) {
+			headers := []qpack.HeaderField{
+				{Name: ":status", Value: "404"},
+				{Name: invalidField, Value: "some-value"},
+			}
+			err := updateResponseFromHeaders(&http.Response{}, headers)
+			Expect(err).To(MatchError(fmt.Sprintf("invalid header field name: %q", invalidField)))
+		},
+		Entry("connection", "connection"),
+		Entry("keep-alive", "keep-alive"),
+		Entry("proxy-connection", "proxy-connection"),
+		Entry("transfer-encoding", "transfer-encoding"),
+		Entry("upgrade", "upgrade"),
+	)
+
+	It("rejects the TE header field, unless it is set to trailers", func() {
 		headers := []qpack.HeaderField{
 			{Name: ":status", Value: "404"},
-			{Name: "transfer-encoding", Value: "chunked"},
+			{Name: "te", Value: "trailers"},
 		}
-		err := updateResponseFromHeaders(&http.Response{}, headers)
-		Expect(err).To(MatchError("invalid header field: Transfer-Encoding"))
+		Expect(updateResponseFromHeaders(&http.Response{}, headers)).To(Succeed())
+		headers = []qpack.HeaderField{
+			{Name: ":status", Value: "404"},
+			{Name: "te", Value: "not-trailers"},
+		}
+		Expect(updateResponseFromHeaders(&http.Response{}, headers)).To(MatchError("invalid TE header field value: \"not-trailers\""))
 	})
 
 	It("parses trailers", func() {
diff --git a/integrationtests/self/http_test.go b/integrationtests/self/http_test.go
index cf978685..cef97ba2 100644
--- a/integrationtests/self/http_test.go
+++ b/integrationtests/self/http_test.go
@@ -765,8 +765,7 @@ var _ = Describe("HTTP tests", func() {
 	It("processes 1xx terminal response", func() {
 		mux.HandleFunc("/101-switch-protocols", func(w http.ResponseWriter, r *http.Request) {
 			defer GinkgoRecover()
-			w.Header().Add("Connection", "upgrade")
-			w.Header().Add("Upgrade", "proto")
+			w.Header().Add("foo", "bar")
 			w.WriteHeader(http.StatusSwitchingProtocols)
 		})
 
@@ -787,8 +786,7 @@ var _ = Describe("HTTP tests", func() {
 		resp, err := client.Do(req)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(resp.StatusCode).To(Equal(http.StatusSwitchingProtocols))
-		Expect(resp.Header).To(HaveKeyWithValue("Connection", []string{"upgrade"}))
-		Expect(resp.Header).To(HaveKeyWithValue("Upgrade", []string{"proto"}))
+		Expect(resp.Header).To(HaveKeyWithValue("Foo", []string{"bar"}))
 		Expect(status).To(Equal(0))
 		Expect(cnt).To(Equal(0))
 	})