[release-branch.go1.11] all: merge release-branch.go1.11-security into release-branch.go1.11

Change-Id: Icf6d0fb76112e5a9045d0c03bf86072cfa694e6c

VERSION

@@ -1 +1 @@
-go1.11.12
+go1.11.13

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>

src/net/http/h2_bundle.go

@@ -3835,10 +3835,11 @@ func (p *http2pipe) Done() <-chan struct{} {
 }
 
 const (
-	http2prefaceTimeout        = 10 * time.Second
+	http2prefaceTimeout         = 10 * time.Second
-	http2firstSettingsTimeout  = 2 * time.Second // should be in-flight with preface anyway
+	http2firstSettingsTimeout   = 2 * time.Second // should be in-flight with preface anyway
-	http2handlerChunkWriteSize = 4 << 10
+	http2handlerChunkWriteSize  = 4 << 10
-	http2defaultMaxStreams     = 250 // TODO: make this 100 as the GFE seems to?
+	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
+// If a frame isn't being written and we need to send one, the best frame
-// to send is selected, preferring first things that aren't
+// to send is selected by writeSched.
-// stream-specific (e.g. ACKing settings), and then finding the
-// highest priority stream.
 //
 // 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 {

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)
 	}

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)
 	}

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
+// If the result is enclosed in square brackets, as literal IPv6 addresses are,
-// IPv6 literal without the square brackets. IPv6 literals may include
+// the square brackets are removed from the result.
-// a zone identifier.
 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 {
+// splitHostPort separates host and port. If the port is not valid, it returns
-	colon := strings.IndexByte(hostport, ':')
+// the entire input as host, and it doesn't check the validity of the host.
-	if colon == -1 {
+// Unlike net.SplitHostPort, but per RFC 3986, it requires ports to be numeric.
-		return hostport
+func splitHostPort(hostport string) (host, port string) {
-	}
+	host = hostport
-	if i := strings.IndexByte(hostport, ']'); i != -1 {
-		return strings.TrimPrefix(hostport[:i], "[")
-	}
-	return hostport[:colon]
-}
 
-func portOnly(hostport string) string {
+	colon := strings.LastIndexByte(host, ':')
-	colon := strings.IndexByte(hostport, ':')
+	if colon != -1 && validOptionalPort(host[colon:]) {
-	if colon == -1 {
+		host, port = host[:colon], host[colon+1:]
-		return ""
 	}
-	if i := strings.Index(hostport, "]:"); i != -1 {
+
-		return hostport[i+len("]:"):]
+	if strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]") {
+		host = host[1 : len(host)-1]
 	}
-	if strings.Contains(hostport, "]") {
+
-		return ""
+	return
-	}
-	return hostport[colon+len(":"):]
 }
 
 // Marshaling interface implementations.

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
+		in   string // URL.Host field
-		want string
+		host string
+		port string
 	}{
-		{"foo.com:80", "foo.com"},
+		{"foo.com:80", "foo.com", "80"},
-		{"foo.com", "foo.com"},
+		{"foo.com", "foo.com", ""},
-		{"FOO.COM", "FOO.COM"}, // no canonicalization (yet?)
+		{"foo.com:", "foo.com", ""},
-		{"1.2.3.4", "1.2.3.4"},
+		{"FOO.COM", "FOO.COM", ""}, // no canonicalization
-		{"1.2.3.4:80", "1.2.3.4"},
+		{"1.2.3.4", "1.2.3.4", ""},
-		{"[1:2:3:4]", "1:2:3:4"},
+		{"1.2.3.4:80", "1.2.3.4", "80"},
-		{"[1:2:3:4]:80", "1:2:3:4"},
+		{"[1:2:3:4]", "1:2:3:4", ""},
-		{"[::1]:80", "::1"},
+		{"[1:2:3:4]:80", "1:2:3:4", "80"},
-	}
+		{"[::1]:80", "::1", "80"},
-	for _, tt := range tests {
+		{"[::1]", "::1", ""},
-		u := &URL{Host: tt.host}
+		{"[::1]:", "::1", ""},
-		got := u.Hostname()
+		{"localhost", "localhost", ""},
-		if got != tt.want {
+		{"localhost:443", "localhost", "443"},
-			t.Errorf("Hostname for Host %q = %q; want %q", tt.host, got, tt.want)
+		{"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", ""},
-}
 
-func TestURLPort(t *testing.T) {
+		// Ensure that even when not valid, Host is one of "Hostname",
-	tests := []struct {
+		// "Hostname:Port", "[Hostname]" or "[Hostname]:Port".
-		host string // URL.Host field
+		// See https://golang.org/issue/29098.
-		want string
+		{"[google.com]:80", "google.com", "80"},
-	}{
+		{"google.com]:80", "google.com]", "80"},
-		{"foo.com", ""},
+		{"google.com:80_invalid_port", "google.com:80_invalid_port", ""},
-		{"foo.com:80", "80"},
+		{"[::1]extra]:80", "::1]extra", "80"},
-		{"1.2.3.4", ""},
+		{"google.com]extra:extra", "google.com]extra:extra", ""},
-		{"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}
+		u := &URL{Host: tt.in}
-		got := u.Port()
+		host, port := u.Hostname(), u.Port()
-		if got != tt.want {
+		if host != tt.host {
-			t.Errorf("Port for Host %q = %q; want %q", tt.host, got, tt.want)
+			t.Errorf("Hostname for Host %q = %q; want %q", tt.in, host, tt.host)
+		}
+		if port != tt.port {
+			t.Errorf("Port for Host %q = %q; want %q", tt.in, port, tt.port)
 		}
 	}
 }