]> Cypherpunks repositories - gostls13.git/commitdiff
net/http: validate transmitted header fields
authorBrad Fitzpatrick <bradfitz@golang.org>
Thu, 31 Mar 2016 03:33:46 +0000 (14:33 +1100)
committerBrad Fitzpatrick <bradfitz@golang.org>
Thu, 31 Mar 2016 04:55:58 +0000 (04:55 +0000)
This makes sure the net/http package never attempts to transmit a
bogus header field key or value and instead fails fast with an error
to the user, rather than relying on the server to maybe return an
error.

It's still possible to use x/net/http2.Transport directly to send
bogus stuff. This change only stops h1 & h2 usage via the net/http
package. A future change will update x/net/http2.

This change also moves some code from request.go to lex.go, which in a
separate future change should be moved so it can be shared with http2
to reduce code bloat.

Updates #14048

Change-Id: I0a44ae1ab357fbfcbe037aa4b5d50669a87f2856
Reviewed-on: https://go-review.googlesource.com/21326
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

src/net/http/clientserver_test.go
src/net/http/lex.go
src/net/http/request.go
src/net/http/transport.go

index 171060b5413affddebfc6d5d8955de096f952723..fdc47db60a83fb7c53b15bf977efed9c85b4e440 100644 (file)
@@ -1045,6 +1045,63 @@ func testTransportGCRequest(t *testing.T, h2, body bool) {
        }
 }
 
+func TestTransportRejectsInvalidHeaders_h1(t *testing.T) {
+       testTransportRejectsInvalidHeaders(t, h1Mode)
+}
+func TestTransportRejectsInvalidHeaders_h2(t *testing.T) {
+       testTransportRejectsInvalidHeaders(t, h2Mode)
+}
+func testTransportRejectsInvalidHeaders(t *testing.T, h2 bool) {
+       defer afterTest(t)
+       cst := newClientServerTest(t, h2, HandlerFunc(func(w ResponseWriter, r *Request) {
+               fmt.Fprintf(w, "Handler saw headers: %q", r.Header)
+       }))
+       defer cst.close()
+       cst.tr.DisableKeepAlives = true
+
+       tests := []struct {
+               key, val string
+               ok       bool
+       }{
+               {"Foo", "capital-key", true}, // verify h2 allows capital keys
+               {"Foo", "foo\x00bar", false}, // \x00 byte in value not allowed
+               {"Foo", "two\nlines", false}, // \n byte in value not allowed
+               {"bogus\nkey", "v", false},   // \n byte also not allowed in key
+               {"A space", "v", false},      // spaces in keys not allowed
+               {"имя", "v", false},          // key must be ascii
+               {"name", "валю", true},       // value may be non-ascii
+               {"", "v", false},             // key must be non-empty
+               {"k", "", true},              // value may be empty
+       }
+       for _, tt := range tests {
+               dialedc := make(chan bool, 1)
+               cst.tr.Dial = func(netw, addr string) (net.Conn, error) {
+                       dialedc <- true
+                       return net.Dial(netw, addr)
+               }
+               req, _ := NewRequest("GET", cst.ts.URL, nil)
+               req.Header[tt.key] = []string{tt.val}
+               res, err := cst.c.Do(req)
+               var body []byte
+               if err == nil {
+                       body, _ = ioutil.ReadAll(res.Body)
+                       res.Body.Close()
+               }
+               var dialed bool
+               select {
+               case <-dialedc:
+                       dialed = true
+               default:
+               }
+
+               if !tt.ok && dialed {
+                       t.Errorf("For key %q, value %q, transport dialed. Expected local failure. Response was: (%v, %v)\nServer replied with: %s", tt.key, tt.val, res, err, body)
+               } else if (err == nil) != tt.ok {
+                       t.Errorf("For key %q, value %q; got err = %v; want ok=%v", tt.key, tt.val, err, tt.ok)
+               }
+       }
+}
+
 type noteCloseConn struct {
        net.Conn
        closeFunc func()
index 52b6481c14e725b2229e91916c1298e55b4a3cff..63d14ec2ecca5bc089a41ced39477bdccf2dee4e 100644 (file)
@@ -181,3 +181,97 @@ func isCTL(b byte) bool {
        const del = 0x7f // a CTL
        return b < ' ' || b == del
 }
+
+func validHeaderName(v string) bool {
+       if len(v) == 0 {
+               return false
+       }
+       for _, r := range v {
+               if !isToken(r) {
+                       return false
+               }
+       }
+       return true
+}
+
+func validHostHeader(h string) bool {
+       // The latests spec is actually this:
+       //
+       // http://tools.ietf.org/html/rfc7230#section-5.4
+       //     Host = uri-host [ ":" port ]
+       //
+       // Where uri-host is:
+       //     http://tools.ietf.org/html/rfc3986#section-3.2.2
+       //
+       // But we're going to be much more lenient for now and just
+       // search for any byte that's not a valid byte in any of those
+       // expressions.
+       for i := 0; i < len(h); i++ {
+               if !validHostByte[h[i]] {
+                       return false
+               }
+       }
+       return true
+}
+
+// See the validHostHeader comment.
+var validHostByte = [256]bool{
+       '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, 'v': true, 'w': true, 'x': true,
+       'y': true, 'z': 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, // sub-delims
+       '$':  true, // sub-delims
+       '%':  true, // pct-encoded (and used in IPv6 zones)
+       '&':  true, // sub-delims
+       '(':  true, // sub-delims
+       ')':  true, // sub-delims
+       '*':  true, // sub-delims
+       '+':  true, // sub-delims
+       ',':  true, // sub-delims
+       '-':  true, // unreserved
+       '.':  true, // unreserved
+       ':':  true, // IPv6address + Host expression's optional port
+       ';':  true, // sub-delims
+       '=':  true, // sub-delims
+       '[':  true,
+       '\'': true, // sub-delims
+       ']':  true,
+       '_':  true, // unreserved
+       '~':  true, // unreserved
+}
+
+// validHeaderValue reports whether v is a valid "field-value" according to
+// http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2 :
+//
+//        message-header = field-name ":" [ field-value ]
+//        field-value    = *( field-content | LWS )
+//        field-content  = <the OCTETs making up the field-value
+//                         and consisting of either *TEXT or combinations
+//                         of token, separators, and quoted-string>
+//
+// http://www.w3.org/Protocols/rfc2616/rfc2616-sec2.html#sec2.2 :
+//
+//        TEXT           = <any OCTET except CTLs,
+//                          but including LWS>
+//        LWS            = [CRLF] 1*( SP | HT )
+//        CTL            = <any US-ASCII control character
+//                         (octets 0 - 31) and DEL (127)>
+func validHeaderValue(v string) bool {
+       for i := 0; i < len(v); i++ {
+               b := v[i]
+               if isCTL(b) && !isLWS(b) {
+                       return false
+               }
+       }
+       return true
+}
index ba487cfa31ac850f2b97229543dcb5f1ed7b52c1..9cf2d2576fbe3e159770d4cf4ca91b3c78df6f89 100644 (file)
@@ -1106,92 +1106,3 @@ func (r *Request) isReplayable() bool {
        }
        return false
 }
-
-func validHostHeader(h string) bool {
-       // The latests spec is actually this:
-       //
-       // http://tools.ietf.org/html/rfc7230#section-5.4
-       //     Host = uri-host [ ":" port ]
-       //
-       // Where uri-host is:
-       //     http://tools.ietf.org/html/rfc3986#section-3.2.2
-       //
-       // But we're going to be much more lenient for now and just
-       // search for any byte that's not a valid byte in any of those
-       // expressions.
-       for i := 0; i < len(h); i++ {
-               if !validHostByte[h[i]] {
-                       return false
-               }
-       }
-       return true
-}
-
-// See the validHostHeader comment.
-var validHostByte = [256]bool{
-       '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, 'v': true, 'w': true, 'x': true,
-       'y': true, 'z': 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, // sub-delims
-       '$':  true, // sub-delims
-       '%':  true, // pct-encoded (and used in IPv6 zones)
-       '&':  true, // sub-delims
-       '(':  true, // sub-delims
-       ')':  true, // sub-delims
-       '*':  true, // sub-delims
-       '+':  true, // sub-delims
-       ',':  true, // sub-delims
-       '-':  true, // unreserved
-       '.':  true, // unreserved
-       ':':  true, // IPv6address + Host expression's optional port
-       ';':  true, // sub-delims
-       '=':  true, // sub-delims
-       '[':  true,
-       '\'': true, // sub-delims
-       ']':  true,
-       '_':  true, // unreserved
-       '~':  true, // unreserved
-}
-
-func validHeaderName(v string) bool {
-       if len(v) == 0 {
-               return false
-       }
-       return strings.IndexFunc(v, isNotToken) == -1
-}
-
-// validHeaderValue reports whether v is a valid "field-value" according to
-// http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2 :
-//
-//        message-header = field-name ":" [ field-value ]
-//        field-value    = *( field-content | LWS )
-//        field-content  = <the OCTETs making up the field-value
-//                         and consisting of either *TEXT or combinations
-//                         of token, separators, and quoted-string>
-//
-// http://www.w3.org/Protocols/rfc2616/rfc2616-sec2.html#sec2.2 :
-//
-//        TEXT           = <any OCTET except CTLs,
-//                          but including LWS>
-//        LWS            = [CRLF] 1*( SP | HT )
-//        CTL            = <any US-ASCII control character
-//                         (octets 0 - 31) and DEL (127)>
-func validHeaderValue(v string) bool {
-       for i := 0; i < len(v); i++ {
-               b := v[i]
-               if isCTL(b) && !isLWS(b) {
-                       return false
-               }
-       }
-       return true
-}
index 774294ff070fc90c18ef275380fb22cbc529daef..b6a1b330146d76d7b8f6302dae8df294a5b4842e 100644 (file)
@@ -274,18 +274,32 @@ func (t *Transport) RoundTrip(req *Request) (*Response, error) {
                req.closeBody()
                return nil, errors.New("http: nil Request.Header")
        }
+       scheme := req.URL.Scheme
+       isHTTP := scheme == "http" || scheme == "https"
+       if isHTTP {
+               for k, vv := range req.Header {
+                       if !validHeaderName(k) {
+                               return nil, fmt.Errorf("net/http: invalid header field name %q", k)
+                       }
+                       for _, v := range vv {
+                               if !validHeaderValue(v) {
+                                       return nil, fmt.Errorf("net/http: invalid header field value %q for key %v", v, k)
+                               }
+                       }
+               }
+       }
        // TODO(bradfitz): switch to atomic.Value for this map instead of RWMutex
        t.altMu.RLock()
-       altRT := t.altProto[req.URL.Scheme]
+       altRT := t.altProto[scheme]
        t.altMu.RUnlock()
        if altRT != nil {
                if resp, err := altRT.RoundTrip(req); err != ErrSkipAltProtocol {
                        return resp, err
                }
        }
-       if s := req.URL.Scheme; s != "http" && s != "https" {
+       if !isHTTP {
                req.closeBody()
-               return nil, &badStringError{"unsupported protocol scheme", s}
+               return nil, &badStringError{"unsupported protocol scheme", scheme}
        }
        if req.Method != "" && !validMethod(req.Method) {
                return nil, fmt.Errorf("net/http: invalid method %q", req.Method)