http2: make Server reject connection-level headers with a 400 response

Fixes golang/go#14214

Change-Id: I474af4735e2135d787e948220a8fcdbba73a2b25
Reviewed-on: https://go-review.googlesource.com/21534
Reviewed-by: Andrew Gerrand <adg@golang.org>

http2/server.go

@@ -1437,6 +1437,8 @@ func (sc *serverConn) processHeaders(f *MetaHeadersFrame) error {
 	if f.Truncated {
 		// Their header list was too long. Send a 431 error.
 		handler = handleHeaderListTooLong
+	} else if err := checkValidHTTP2Request(req); err != nil {
+		handler = new400Handler(err)
 	}
 
 	go sc.runHandler(rw, req, handler)
@@ -2180,3 +2182,34 @@ func foreachHeaderElement(v string, fn func(string)) {
 		}
 	}
 }
+
+// From http://httpwg.org/specs/rfc7540.html#rfc.section.8.1.2.2
+var connHeaders = []string{
+	"Connection",
+	"Keep-Alive",
+	"Proxy-Connection",
+	"Transfer-Encoding",
+	"Upgrade",
+}
+
+// checkValidHTTP2Request checks whether req is a valid HTTP/2 request,
+// per RFC 7540 Section 8.1.2.2.
+// The returned error is reported to users.
+func checkValidHTTP2Request(req *http.Request) error {
+	for _, h := range connHeaders {
+		if _, ok := req.Header[h]; ok {
+			return fmt.Errorf("request header %q is not valid in HTTP/2", h)
+		}
+	}
+	te := req.Header["Te"]
+	if len(te) > 0 && (len(te) > 1 || (te[0] != "trailers" && te[0] != "")) {
+		return errors.New(`request header "TE" may only be "trailers" in HTTP/2`)
+	}
+	return nil
+}
+
+func new400Handler(err error) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		http.Error(w, err.Error(), http.StatusBadRequest)
+	}
+}

http2/server_test.go

@@ -3156,6 +3156,27 @@ func TestServerHandleCustomConn(t *testing.T) {
 	}
 }
 
+// golang.org/issue/14214
+func TestServer_Rejects_ConnHeaders(t *testing.T) {
+	testServerResponse(t, func(w http.ResponseWriter, r *http.Request) error {
+		t.Errorf("should not get to Handler")
+		return nil
+	}, func(st *serverTester) {
+		st.bodylessReq1("connection", "foo")
+		hf := st.wantHeaders()
+		goth := st.decodeHeader(hf.HeaderBlockFragment())
+		wanth := [][2]string{
+			{":status", "400"},
+			{"content-type", "text/plain; charset=utf-8"},
+			{"x-content-type-options", "nosniff"},
+			{"content-length", "51"},
+		}
+		if !reflect.DeepEqual(goth, wanth) {
+			t.Errorf("Got headers %v; want %v", goth, wanth)
+		}
+	})
+}
+
 type hpackEncoder struct {
 	enc *hpack.Encoder
 	buf bytes.Buffer
@@ -3179,3 +3200,45 @@ func (he *hpackEncoder) encodeHeaderRaw(t *testing.T, headers ...string) []byte
 	}
 	return he.buf.Bytes()
 }
+
+func TestCheckValidHTTP2Request(t *testing.T) {
+	tests := []struct {
+		req  *http.Request
+		want error
+	}{
+		{
+			req:  &http.Request{Header: http.Header{"Te": {"trailers"}}},
+			want: nil,
+		},
+		{
+			req:  &http.Request{Header: http.Header{"Te": {"trailers", "bogus"}}},
+			want: errors.New(`request header "TE" may only be "trailers" in HTTP/2`),
+		},
+		{
+			req:  &http.Request{Header: http.Header{"Foo": {""}}},
+			want: nil,
+		},
+		{
+			req:  &http.Request{Header: http.Header{"Connection": {""}}},
+			want: errors.New(`request header "Connection" is not valid in HTTP/2`),
+		},
+		{
+			req:  &http.Request{Header: http.Header{"Proxy-Connection": {""}}},
+			want: errors.New(`request header "Proxy-Connection" is not valid in HTTP/2`),
+		},
+		{
+			req:  &http.Request{Header: http.Header{"Keep-Alive": {""}}},
+			want: errors.New(`request header "Keep-Alive" is not valid in HTTP/2`),
+		},
+		{
+			req:  &http.Request{Header: http.Header{"Upgrade": {""}}},
+			want: errors.New(`request header "Upgrade" is not valid in HTTP/2`),
+		},
+	}
+	for i, tt := range tests {
+		got := checkValidHTTP2Request(tt.req)
+		if !reflect.DeepEqual(got, tt.want) {
+			t.Errorf("%d. checkValidHTTP2Request = %v; want %v", i, got, tt.want)
+		}
+	}
+}