summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFilippo Valsorda <filippo@golang.org>2019-08-13 16:44:04 -0400
committerFilippo Valsorda <filippo@golang.org>2019-08-13 16:44:04 -0400
commit4f8e7223f93676f2e779fa581536756f37bbe9a9 (patch)
tree59035c0ac6a7476cbb3b0f7af9667c74c8557f85
parent845e947ae34f8895d58e8da13f9b83d5505eec93 (diff)
parentb2967c0e5c5271bb4469e1f615fb85879ebd8a57 (diff)
downloadgo-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.html7
-rw-r--r--src/net/http/h2_bundle.go54
-rw-r--r--src/net/http/transport.go2
-rw-r--r--src/net/http/transport_test.go2
-rw-r--r--src/net/url/url.go54
-rw-r--r--src/net/url/url_test.go76
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)
}
}
}