http2: reject more trailer values

Updates golang/go#14188

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

http2/server.go

@@ -1474,6 +1474,12 @@ func (st *stream) processTrailerHeaders(f *MetaHeadersFrame) error {
 	if st.trailer != nil {
 		for _, hf := range f.RegularFields() {
 			key := sc.canonicalHeader(hf.Name)
+			if !ValidTrailerHeader(key) {
+				// TODO: send more details to the peer somehow. But http2 has
+				// no way to send debug data at a stream level. Discuss with
+				// HTTP folk.
+				return StreamError{st.id, ErrCodeProtocol}
+			}
 			st.trailer[key] = append(st.trailer[key], hf.Value)
 		}
 	}
@@ -1902,9 +1908,9 @@ func (rws *responseWriterState) hasTrailers() bool { return len(rws.trailers) !=
 // written in the trailers at the end of the response.
 func (rws *responseWriterState) declareTrailer(k string) {
 	k = http.CanonicalHeaderKey(k)
-	switch k {
-	case "Transfer-Encoding", "Content-Length", "Trailer":
+	if !ValidTrailerHeader(k) {
 		// Forbidden by RFC 2616 14.40.
+		rws.conn.logf("ignoring invalid trailer %q", k)
 		return
 	}
 	if !strSliceContains(rws.trailers, k) {
@@ -2227,3 +2233,38 @@ func new400Handler(err error) http.HandlerFunc {
 		http.Error(w, err.Error(), http.StatusBadRequest)
 	}
 }
+
+// ValidTrailerHeader reports whether name is a valid header field name to appear
+// in trailers.
+// See: http://tools.ietf.org/html/rfc7230#section-4.1.2
+func ValidTrailerHeader(name string) bool {
+	name = http.CanonicalHeaderKey(name)
+	if strings.HasPrefix(name, "If-") || badTrailer[name] {
+		return false
+	}
+	return true
+}
+
+var badTrailer = map[string]bool{
+	"Authorization":       true,
+	"Cache-Control":       true,
+	"Connection":          true,
+	"Content-Encoding":    true,
+	"Content-Length":      true,
+	"Content-Range":       true,
+	"Content-Type":        true,
+	"Expect":              true,
+	"Host":                true,
+	"Keep-Alive":          true,
+	"Max-Forwards":        true,
+	"Pragma":              true,
+	"Proxy-Authenticate":  true,
+	"Proxy-Authorization": true,
+	"Proxy-Connection":    true,
+	"Range":               true,
+	"Realm":               true,
+	"Te":                  true,
+	"Trailer":             true,
+	"Transfer-Encoding":   true,
+	"Www-Authenticate":    true,
+}

http2/server_test.go

@@ -2660,13 +2660,11 @@ func testServerWritesTrailers(t *testing.T, withFlush bool) {
 	testServerResponse(t, func(w http.ResponseWriter, r *http.Request) error {
 		w.Header().Set("Trailer", "Server-Trailer-A, Server-Trailer-B")
 		w.Header().Add("Trailer", "Server-Trailer-C")
-
-		// TODO: decide if the server should filter these while
-		// writing the Trailer header in the response. Currently it
-		// appears net/http doesn't do this for http/1.1
 		w.Header().Add("Trailer", "Transfer-Encoding, Content-Length, Trailer") // filtered
+
+		// Regular headers:
 		w.Header().Set("Foo", "Bar")
-		w.Header().Set("Content-Length", "5")
+		w.Header().Set("Content-Length", "5") // len("Hello")
 
 		io.WriteString(w, "Hello")
 		if withFlush {
@@ -2681,6 +2679,8 @@ func testServerWritesTrailers(t *testing.T, withFlush bool) {
 		// otherwise-invalid "Trailer:" prefix:
 		w.Header().Set("Trailer:Post-Header-Trailer", "hi1")
 		w.Header().Set("Trailer:post-header-trailer2", "hi2")
+		w.Header().Set("Trailer:Range", "invalid")
+		w.Header().Set("Trailer:Foo\x01Bogus", "invalid")
 		w.Header().Set("Transfer-Encoding", "should not be included; Forbidden by RFC 2616 14.40")
 		w.Header().Set("Content-Length", "should not be included; Forbidden by RFC 2616 14.40")
 		w.Header().Set("Trailer", "should not be included; Forbidden by RFC 2616 14.40")