Commit e8e1928b authored by Brad Fitzpatrick's avatar Brad Fitzpatrick

net/http: update http2 to check header values, move from vendor to internal

Updates x/net/http2 to git rev b2ed34f for https://golang.org/cl/18727

Updates #14029 (fixes it enough for Go 1.6)
Fixes #13961

Change-Id: Id301247545507671f4e79df0e7c6ec9c421d5a7c
Reviewed-on: https://go-review.googlesource.com/18728
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: default avatarAndrew Gerrand <adg@golang.org>
parent cd9fc3eb
...@@ -300,6 +300,7 @@ func TestShellSafety(t *testing.T) { ...@@ -300,6 +300,7 @@ func TestShellSafety(t *testing.T) {
} }
func TestImportVendor(t *testing.T) { func TestImportVendor(t *testing.T) {
t.Skip("skipping; hpack has moved to internal for now; golang.org/issue/14047")
testenv.MustHaveGoBuild(t) // really must just have source testenv.MustHaveGoBuild(t) // really must just have source
ctxt := Default ctxt := Default
ctxt.GOPATH = "" ctxt.GOPATH = ""
......
...@@ -358,7 +358,7 @@ var pkgDeps = map[string][]string{ ...@@ -358,7 +358,7 @@ var pkgDeps = map[string][]string{
"L4", "NET", "OS", "L4", "NET", "OS",
"compress/gzip", "crypto/tls", "mime/multipart", "runtime/debug", "compress/gzip", "crypto/tls", "mime/multipart", "runtime/debug",
"net/http/internal", "net/http/internal",
"golang.org/x/net/http2/hpack", "internal/golang.org/x/net/http2/hpack",
}, },
"net/http/internal": {"L4"}, "net/http/internal": {"L4"},
......
...@@ -24,6 +24,7 @@ import ( ...@@ -24,6 +24,7 @@ import (
"encoding/binary" "encoding/binary"
"errors" "errors"
"fmt" "fmt"
"internal/golang.org/x/net/http2/hpack"
"io" "io"
"io/ioutil" "io/ioutil"
"log" "log"
...@@ -37,8 +38,6 @@ import ( ...@@ -37,8 +38,6 @@ import (
"strings" "strings"
"sync" "sync"
"time" "time"
"golang.org/x/net/http2/hpack"
) )
// ClientConnPool manages a pool of HTTP/2 client connections. // ClientConnPool manages a pool of HTTP/2 client connections.
...@@ -2065,13 +2064,60 @@ func (s http2SettingID) String() string { ...@@ -2065,13 +2064,60 @@ func (s http2SettingID) String() string {
return fmt.Sprintf("UNKNOWN_SETTING_%d", uint16(s)) return fmt.Sprintf("UNKNOWN_SETTING_%d", uint16(s))
} }
func http2validHeader(v string) bool { var (
http2errInvalidHeaderFieldName = errors.New("http2: invalid header field name")
http2errInvalidHeaderFieldValue = errors.New("http2: invalid header field value")
)
// validHeaderFieldName reports whether v is a valid header field name (key).
// RFC 7230 says:
// header-field = field-name ":" OWS field-value OWS
// field-name = token
// tchar = "!" / "#" / "$" / "%" / "&" / "'" / "*" / "+" / "-" / "." /
// "^" / "_" / "
// Further, http2 says:
// "Just as in HTTP/1.x, header field names are strings of ASCII
// characters that are compared in a case-insensitive
// fashion. However, header field names MUST be converted to
// lowercase prior to their encoding in HTTP/2. "
func http2validHeaderFieldName(v string) bool {
if len(v) == 0 { if len(v) == 0 {
return false return false
} }
for _, r := range v { for _, r := range v {
if int(r) >= len(http2isTokenTable) || ('A' <= r && r <= 'Z') {
return false
}
if !http2isTokenTable[byte(r)] {
return false
}
}
return true
}
if r >= 127 || ('A' <= r && r <= 'Z') { // validHeaderFieldValue reports whether v is a valid header field value.
//
// RFC 7230 says:
// field-content = field-vchar [ 1*( SP / HTAB ) field-vchar ]
// field-vchar = VCHAR / obs-text
// obs-text = %x80-FF
// VCHAR = "any visible [USASCII] character"
//
// http2 further says: "Similarly, HTTP/2 allows header field values
// that are not valid. While most of the values that can be encoded
// will not alter header field parsing, carriage return (CR, ASCII
// 0xd), line feed (LF, ASCII 0xa), and the zero character (NUL, ASCII
// 0x0) might be exploited by an attacker if they are translated
// verbatim. Any request or response that contains a character not
// permitted in a header field value MUST be treated as malformed
// (Section 8.1.2.6). Valid characters are defined by the
// field-content ABNF rule in Section 3.2 of [RFC7230]."
//
// This function does not (yet?) properly handle the rejection of
// strings that begin or end with SP or HTAB.
func http2validHeaderFieldValue(v string) bool {
for i := 0; i < len(v); i++ {
if b := v[i]; b < ' ' && b != '\t' {
return false return false
} }
} }
...@@ -2202,6 +2248,86 @@ func (e *http2httpError) Temporary() bool { return true } ...@@ -2202,6 +2248,86 @@ func (e *http2httpError) Temporary() bool { return true }
var http2errTimeout error = &http2httpError{msg: "http2: timeout awaiting response headers", timeout: true} var http2errTimeout error = &http2httpError{msg: "http2: timeout awaiting response headers", timeout: true}
var http2isTokenTable = [127]bool{
'!': true,
'#': true,
'$': true,
'%': true,
'&': true,
'\'': true,
'*': true,
'+': true,
'-': true,
'.': true,
'0': true,
'1': true,
'2': true,
'3': true,
'4': true,
'5': true,
'6': true,
'7': true,
'8': true,
'9': true,
'A': true,
'B': true,
'C': true,
'D': true,
'E': true,
'F': true,
'G': true,
'H': true,
'I': true,
'J': true,
'K': true,
'L': true,
'M': true,
'N': true,
'O': true,
'P': true,
'Q': true,
'R': true,
'S': true,
'T': true,
'U': true,
'W': true,
'V': true,
'X': true,
'Y': true,
'Z': true,
'^': true,
'_': true,
'`': true,
'a': true,
'b': true,
'c': true,
'd': true,
'e': true,
'f': true,
'g': true,
'h': true,
'i': true,
'j': true,
'k': true,
'l': true,
'm': true,
'n': true,
'o': true,
'p': true,
'q': true,
'r': true,
's': true,
't': true,
'u': true,
'v': true,
'w': true,
'x': true,
'y': true,
'z': true,
'|': true,
'~': true,
}
// pipe is a goroutine-safe io.Reader/io.Writer pair. It's like // pipe is a goroutine-safe io.Reader/io.Writer pair. It's like
// io.Pipe except there are no PipeReader/PipeWriter halves, and the // io.Pipe except there are no PipeReader/PipeWriter halves, and the
// underlying buffer is an interface. (io.Pipe is always unbuffered) // underlying buffer is an interface. (io.Pipe is always unbuffered)
...@@ -2741,7 +2867,7 @@ func (sc *http2serverConn) onNewHeaderField(f hpack.HeaderField) { ...@@ -2741,7 +2867,7 @@ func (sc *http2serverConn) onNewHeaderField(f hpack.HeaderField) {
sc.vlogf("http2: server decoded %v", f) sc.vlogf("http2: server decoded %v", f)
} }
switch { switch {
case !http2validHeader(f.Name): case !http2validHeaderFieldValue(f.Value):
sc.req.invalidHeader = true sc.req.invalidHeader = true
case strings.HasPrefix(f.Name, ":"): case strings.HasPrefix(f.Name, ":"):
if sc.req.sawRegularHeader { if sc.req.sawRegularHeader {
...@@ -2771,6 +2897,8 @@ func (sc *http2serverConn) onNewHeaderField(f hpack.HeaderField) { ...@@ -2771,6 +2897,8 @@ func (sc *http2serverConn) onNewHeaderField(f hpack.HeaderField) {
return return
} }
*dst = f.Value *dst = f.Value
case !http2validHeaderFieldName(f.Name):
sc.req.invalidHeader = true
default: default:
sc.req.sawRegularHeader = true sc.req.sawRegularHeader = true
sc.req.header.Add(sc.canonicalHeader(f.Name), f.Value) sc.req.header.Add(sc.canonicalHeader(f.Name), f.Value)
...@@ -2789,10 +2917,10 @@ func (st *http2stream) onNewTrailerField(f hpack.HeaderField) { ...@@ -2789,10 +2917,10 @@ func (st *http2stream) onNewTrailerField(f hpack.HeaderField) {
sc.vlogf("http2: server decoded trailer %v", f) sc.vlogf("http2: server decoded trailer %v", f)
} }
switch { switch {
case !http2validHeader(f.Name): case strings.HasPrefix(f.Name, ":"):
sc.req.invalidHeader = true sc.req.invalidHeader = true
return return
case strings.HasPrefix(f.Name, ":"): case !http2validHeaderFieldName(f.Name) || !http2validHeaderFieldValue(f.Value):
sc.req.invalidHeader = true sc.req.invalidHeader = true
return return
default: default:
...@@ -5697,7 +5825,6 @@ func (cc *http2ClientConn) writeStreamReset(streamID uint32, code http2ErrCode, ...@@ -5697,7 +5825,6 @@ func (cc *http2ClientConn) writeStreamReset(streamID uint32, code http2ErrCode,
var ( var (
http2errResponseHeaderListSize = errors.New("http2: response header list larger than advertised limit") http2errResponseHeaderListSize = errors.New("http2: response header list larger than advertised limit")
http2errInvalidHeaderKey = errors.New("http2: invalid header key")
http2errPseudoTrailers = errors.New("http2: invalid pseudo header in trailers") http2errPseudoTrailers = errors.New("http2: invalid pseudo header in trailers")
) )
...@@ -5714,8 +5841,8 @@ func (rl *http2clientConnReadLoop) checkHeaderField(f hpack.HeaderField) bool { ...@@ -5714,8 +5841,8 @@ func (rl *http2clientConnReadLoop) checkHeaderField(f hpack.HeaderField) bool {
return false return false
} }
if !http2validHeader(f.Name) { if !http2validHeaderFieldValue(f.Value) {
rl.reqMalformed = http2errInvalidHeaderKey rl.reqMalformed = http2errInvalidHeaderFieldValue
return false return false
} }
...@@ -5726,6 +5853,10 @@ func (rl *http2clientConnReadLoop) checkHeaderField(f hpack.HeaderField) bool { ...@@ -5726,6 +5853,10 @@ func (rl *http2clientConnReadLoop) checkHeaderField(f hpack.HeaderField) bool {
return false return false
} }
} else { } else {
if !http2validHeaderFieldName(f.Name) {
rl.reqMalformed = http2errInvalidHeaderFieldName
return false
}
rl.sawRegHeader = true rl.sawRegHeader = true
} }
......
This file needs to exist because the vendor directory needs
to exist for some go/build tests to pass, and git can't track
empty directories.
In Go 1.7 we'll use this directory again. (In Go 1.6 we tried but
reverted).
See http://golang.org/issue/14047 for details.
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment