diff options
author | Filippo Valsorda <filippo@golang.org> | 2019-08-13 16:44:04 -0400 |
---|---|---|
committer | Filippo Valsorda <filippo@golang.org> | 2019-08-13 16:44:04 -0400 |
commit | 4f8e7223f93676f2e779fa581536756f37bbe9a9 (patch) | |
tree | 59035c0ac6a7476cbb3b0f7af9667c74c8557f85 | |
parent | 845e947ae34f8895d58e8da13f9b83d5505eec93 (diff) | |
parent | b2967c0e5c5271bb4469e1f615fb85879ebd8a57 (diff) | |
download | go-git-dev.boringcrypto.go1.11.tar.gz |
[dev.boringcrypto.go1.11] all: merge go1.11.13 into dev.boringcrypto.go1.11dev.boringcrypto.go1.11
Change-Id: I1d19c9f720ba936213448bbbc22d795c8166eebc
-rw-r--r-- | doc/devel/release.html | 7 | ||||
-rw-r--r-- | src/net/http/h2_bundle.go | 54 | ||||
-rw-r--r-- | src/net/http/transport.go | 2 | ||||
-rw-r--r-- | src/net/http/transport_test.go | 2 | ||||
-rw-r--r-- | src/net/url/url.go | 54 | ||||
-rw-r--r-- | src/net/url/url_test.go | 76 |
6 files changed, 121 insertions, 74 deletions
diff --git a/doc/devel/release.html b/doc/devel/release.html index 69a5741cbb..c24de8e344 100644 --- a/doc/devel/release.html +++ b/doc/devel/release.html @@ -120,6 +120,13 @@ See the <a href="https://github.com/golang/go/issues?q=milestone%3AGo1.11.12">Go 1.11.12 milestone</a> on our issue tracker for details. </p> +<p> +go1.11.13 (released 2019/08/13) includes security fixes to the +<code>net/http</code> and <code>net/url</code> packages. +See the <a href="https://github.com/golang/go/issues?q=milestone%3AGo1.11.13">Go +1.11.13 milestone</a> on our issue tracker for details. +</p> + <h2 id="go1.10">go1.10 (released 2018/02/16)</h2> <p> diff --git a/src/net/http/h2_bundle.go b/src/net/http/h2_bundle.go index 2cd2b86df2..61824950f4 100644 --- a/src/net/http/h2_bundle.go +++ b/src/net/http/h2_bundle.go @@ -3835,10 +3835,11 @@ func (p *http2pipe) Done() <-chan struct{} { } const ( - http2prefaceTimeout = 10 * time.Second - http2firstSettingsTimeout = 2 * time.Second // should be in-flight with preface anyway - http2handlerChunkWriteSize = 4 << 10 - http2defaultMaxStreams = 250 // TODO: make this 100 as the GFE seems to? + http2prefaceTimeout = 10 * time.Second + http2firstSettingsTimeout = 2 * time.Second // should be in-flight with preface anyway + http2handlerChunkWriteSize = 4 << 10 + http2defaultMaxStreams = 250 // TODO: make this 100 as the GFE seems to? + http2maxQueuedControlFrames = 10000 ) var ( @@ -3946,6 +3947,15 @@ func (s *http2Server) maxConcurrentStreams() uint32 { return http2defaultMaxStreams } +// maxQueuedControlFrames is the maximum number of control frames like +// SETTINGS, PING and RST_STREAM that will be queued for writing before +// the connection is closed to prevent memory exhaustion attacks. +func (s *http2Server) maxQueuedControlFrames() int { + // TODO: if anybody asks, add a Server field, and remember to define the + // behavior of negative values. + return http2maxQueuedControlFrames +} + type http2serverInternalState struct { mu sync.Mutex activeConns map[*http2serverConn]struct{} @@ -4254,6 +4264,7 @@ type http2serverConn struct { sawFirstSettings bool // got the initial SETTINGS frame after the preface needToSendSettingsAck bool unackedSettings int // how many SETTINGS have we sent without ACKs? + queuedControlFrames int // control frames in the writeSched queue clientMaxStreams uint32 // SETTINGS_MAX_CONCURRENT_STREAMS from client (our PUSH_PROMISE limit) advMaxStreams uint32 // our SETTINGS_MAX_CONCURRENT_STREAMS advertised the client curClientStreams uint32 // number of open streams initiated by the client @@ -4644,6 +4655,14 @@ func (sc *http2serverConn) serve() { } } + // If the peer is causing us to generate a lot of control frames, + // but not reading them from us, assume they are trying to make us + // run out of memory. + if sc.queuedControlFrames > sc.srv.maxQueuedControlFrames() { + sc.vlogf("http2: too many control frames in send queue, closing connection") + return + } + // Start the shutdown timer after sending a GOAWAY. When sending GOAWAY // with no error code (graceful shutdown), don't start the timer until // all open streams have been completed. @@ -4845,6 +4864,14 @@ func (sc *http2serverConn) writeFrame(wr http2FrameWriteRequest) { } if !ignoreWrite { + if wr.isControl() { + sc.queuedControlFrames++ + // For extra safety, detect wraparounds, which should not happen, + // and pull the plug. + if sc.queuedControlFrames < 0 { + sc.conn.Close() + } + } sc.writeSched.Push(wr) } sc.scheduleFrameWrite() @@ -4962,10 +4989,8 @@ func (sc *http2serverConn) wroteFrame(res http2frameWriteResult) { // If a frame is already being written, nothing happens. This will be called again // when the frame is done being written. // -// If a frame isn't being written we need to send one, the best frame -// to send is selected, preferring first things that aren't -// stream-specific (e.g. ACKing settings), and then finding the -// highest priority stream. +// If a frame isn't being written and we need to send one, the best frame +// to send is selected by writeSched. // // If a frame isn't being written and there's nothing else to send, we // flush the write buffer. @@ -4993,6 +5018,9 @@ func (sc *http2serverConn) scheduleFrameWrite() { } if !sc.inGoAway || sc.goAwayCode == http2ErrCodeNo { if wr, ok := sc.writeSched.Pop(); ok { + if wr.isControl() { + sc.queuedControlFrames-- + } sc.startFrameWrite(wr) continue } @@ -5285,6 +5313,8 @@ func (sc *http2serverConn) processSettings(f *http2SettingsFrame) error { if err := f.ForeachSetting(sc.processSetting); err != nil { return err } + // TODO: judging by RFC 7540, Section 6.5.3 each SETTINGS frame should be + // acknowledged individually, even if multiple are received before the ACK. sc.needToSendSettingsAck = true sc.scheduleFrameWrite() return nil @@ -9476,7 +9506,7 @@ type http2WriteScheduler interface { // Pop dequeues the next frame to write. Returns false if no frames can // be written. Frames with a given wr.StreamID() are Pop'd in the same - // order they are Push'd. + // order they are Push'd. No frames should be discarded except by CloseStream. Pop() (wr http2FrameWriteRequest, ok bool) } @@ -9520,6 +9550,12 @@ func (wr http2FrameWriteRequest) StreamID() uint32 { return wr.stream.id } +// isControl reports whether wr is a control frame for MaxQueuedControlFrames +// purposes. That includes non-stream frames and RST_STREAM frames. +func (wr http2FrameWriteRequest) isControl() bool { + return wr.stream == nil +} + // DataSize returns the number of flow control bytes that must be consumed // to write this entire frame. This is 0 for non-DATA frames. func (wr http2FrameWriteRequest) DataSize() int { diff --git a/src/net/http/transport.go b/src/net/http/transport.go index 40947baf87..0a9812eeda 100644 --- a/src/net/http/transport.go +++ b/src/net/http/transport.go @@ -640,6 +640,8 @@ func resetProxyConfig() { } func (t *Transport) connectMethodForRequest(treq *transportRequest) (cm connectMethod, err error) { + // TODO: the validPort check is redundant after CL 189258, as url.URL.Port + // only returns valid ports now. golang.org/issue/33600 if port := treq.URL.Port(); !validPort(port) { return cm, fmt.Errorf("invalid URL port %q", port) } diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go index aa8beb9357..b2036dfc24 100644 --- a/src/net/http/transport_test.go +++ b/src/net/http/transport_test.go @@ -4111,7 +4111,7 @@ func TestTransportRejectsAlphaPort(t *testing.T) { t.Fatalf("got %#v; want *url.Error", err) } got := ue.Err.Error() - want := `invalid URL port "123foo"` + want := `invalid port ":123foo" after host` if got != want { t.Errorf("got error %q; want %q", got, want) } diff --git a/src/net/url/url.go b/src/net/url/url.go index 8d2a856699..b13677ca8a 100644 --- a/src/net/url/url.go +++ b/src/net/url/url.go @@ -636,6 +636,11 @@ func parseHost(host string) (string, error) { } return host1 + host2 + host3, nil } + } else if i := strings.LastIndex(host, ":"); i != -1 { + colonPort := host[i:] + if !validOptionalPort(colonPort) { + return "", fmt.Errorf("invalid port %q after host", colonPort) + } } var err error @@ -1033,44 +1038,39 @@ func (u *URL) RequestURI() string { return result } -// Hostname returns u.Host, without any port number. +// Hostname returns u.Host, stripping any valid port number if present. // -// If Host is an IPv6 literal with a port number, Hostname returns the -// IPv6 literal without the square brackets. IPv6 literals may include -// a zone identifier. +// If the result is enclosed in square brackets, as literal IPv6 addresses are, +// the square brackets are removed from the result. func (u *URL) Hostname() string { - return stripPort(u.Host) + host, _ := splitHostPort(u.Host) + return host } // Port returns the port part of u.Host, without the leading colon. -// If u.Host doesn't contain a port, Port returns an empty string. +// +// If u.Host doesn't contain a valid numeric port, Port returns an empty string. func (u *URL) Port() string { - return portOnly(u.Host) + _, port := splitHostPort(u.Host) + return port } -func stripPort(hostport string) string { - colon := strings.IndexByte(hostport, ':') - if colon == -1 { - return hostport - } - if i := strings.IndexByte(hostport, ']'); i != -1 { - return strings.TrimPrefix(hostport[:i], "[") - } - return hostport[:colon] -} +// splitHostPort separates host and port. If the port is not valid, it returns +// the entire input as host, and it doesn't check the validity of the host. +// Unlike net.SplitHostPort, but per RFC 3986, it requires ports to be numeric. +func splitHostPort(hostport string) (host, port string) { + host = hostport -func portOnly(hostport string) string { - colon := strings.IndexByte(hostport, ':') - if colon == -1 { - return "" - } - if i := strings.Index(hostport, "]:"); i != -1 { - return hostport[i+len("]:"):] + colon := strings.LastIndexByte(host, ':') + if colon != -1 && validOptionalPort(host[colon:]) { + host, port = host[:colon], host[colon+1:] } - if strings.Contains(hostport, "]") { - return "" + + if strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]") { + host = host[1 : len(host)-1] } - return hostport[colon+len(":"):] + + return } // Marshaling interface implementations. diff --git a/src/net/url/url_test.go b/src/net/url/url_test.go index 369ea6cbd2..babdaec0af 100644 --- a/src/net/url/url_test.go +++ b/src/net/url/url_test.go @@ -422,10 +422,10 @@ var urltests = []URLTest{ }, // worst case host, still round trips { - "scheme://!$&'()*+,;=hello!:port/path", + "scheme://!$&'()*+,;=hello!:1/path", &URL{ Scheme: "scheme", - Host: "!$&'()*+,;=hello!:port", + Host: "!$&'()*+,;=hello!:1", Path: "/path", }, "", @@ -1420,11 +1420,13 @@ func TestParseErrors(t *testing.T) { {"http://[::1]", false}, {"http://[::1]:80", false}, {"http://[::1]:namedport", true}, // rfc3986 3.2.3 + {"http://x:namedport", true}, // rfc3986 3.2.3 {"http://[::1]/", false}, {"http://[::1]a", true}, {"http://[::1]%23", true}, {"http://[::1%25en0]", false}, // valid zone id {"http://[::1]:", false}, // colon, but no port OK + {"http://x:", false}, // colon, but no port OK {"http://[::1]:%38%30", true}, // not allowed: % encoding only for non-ASCII {"http://[::1%25%41]", false}, // RFC 6874 allows over-escaping in zone {"http://[%10::1]", true}, // no %xx escapes in IP address @@ -1616,46 +1618,46 @@ func TestURLErrorImplementsNetError(t *testing.T) { } } -func TestURLHostname(t *testing.T) { +func TestURLHostnameAndPort(t *testing.T) { tests := []struct { - host string // URL.Host field - want string + in string // URL.Host field + host string + port string }{ - {"foo.com:80", "foo.com"}, - {"foo.com", "foo.com"}, - {"FOO.COM", "FOO.COM"}, // no canonicalization (yet?) - {"1.2.3.4", "1.2.3.4"}, - {"1.2.3.4:80", "1.2.3.4"}, - {"[1:2:3:4]", "1:2:3:4"}, - {"[1:2:3:4]:80", "1:2:3:4"}, - {"[::1]:80", "::1"}, + {"foo.com:80", "foo.com", "80"}, + {"foo.com", "foo.com", ""}, + {"foo.com:", "foo.com", ""}, + {"FOO.COM", "FOO.COM", ""}, // no canonicalization + {"1.2.3.4", "1.2.3.4", ""}, + {"1.2.3.4:80", "1.2.3.4", "80"}, + {"[1:2:3:4]", "1:2:3:4", ""}, + {"[1:2:3:4]:80", "1:2:3:4", "80"}, + {"[::1]:80", "::1", "80"}, + {"[::1]", "::1", ""}, + {"[::1]:", "::1", ""}, + {"localhost", "localhost", ""}, + {"localhost:443", "localhost", "443"}, + {"some.super.long.domain.example.org:8080", "some.super.long.domain.example.org", "8080"}, + {"[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:17000", "2001:0db8:85a3:0000:0000:8a2e:0370:7334", "17000"}, + {"[2001:0db8:85a3:0000:0000:8a2e:0370:7334]", "2001:0db8:85a3:0000:0000:8a2e:0370:7334", ""}, + + // Ensure that even when not valid, Host is one of "Hostname", + // "Hostname:Port", "[Hostname]" or "[Hostname]:Port". + // See https://golang.org/issue/29098. + {"[google.com]:80", "google.com", "80"}, + {"google.com]:80", "google.com]", "80"}, + {"google.com:80_invalid_port", "google.com:80_invalid_port", ""}, + {"[::1]extra]:80", "::1]extra", "80"}, + {"google.com]extra:extra", "google.com]extra:extra", ""}, } for _, tt := range tests { - u := &URL{Host: tt.host} - got := u.Hostname() - if got != tt.want { - t.Errorf("Hostname for Host %q = %q; want %q", tt.host, got, tt.want) + u := &URL{Host: tt.in} + host, port := u.Hostname(), u.Port() + if host != tt.host { + t.Errorf("Hostname for Host %q = %q; want %q", tt.in, host, tt.host) } - } -} - -func TestURLPort(t *testing.T) { - tests := []struct { - host string // URL.Host field - want string - }{ - {"foo.com", ""}, - {"foo.com:80", "80"}, - {"1.2.3.4", ""}, - {"1.2.3.4:80", "80"}, - {"[1:2:3:4]", ""}, - {"[1:2:3:4]:80", "80"}, - } - for _, tt := range tests { - u := &URL{Host: tt.host} - got := u.Port() - if got != tt.want { - t.Errorf("Port for Host %q = %q; want %q", tt.host, got, tt.want) + if port != tt.port { + t.Errorf("Port for Host %q = %q; want %q", tt.in, port, tt.port) } } } |