diff -Nru golang-golang-x-net-dev-0.0+git20190724.ca1201d+dfsg/debian/changelog golang-golang-x-net-dev-0.0+git20190811.74dc4d7+dfsg/debian/changelog --- golang-golang-x-net-dev-0.0+git20190724.ca1201d+dfsg/debian/changelog 2019-07-28 08:08:05.000000000 +0000 +++ golang-golang-x-net-dev-0.0+git20190811.74dc4d7+dfsg/debian/changelog 2019-08-17 12:02:42.000000000 +0000 @@ -1,3 +1,11 @@ +golang-golang-x-net-dev (1:0.0+git20190811.74dc4d7+dfsg-1) unstable; urgency=high + + * Team upload. + * New upstream version 0.0+git20190811.74dc4d7+dfsg + - Include fixes for CVE-2019-9512 and CVE-2019-9514 + + -- Dr. Tobias Quathamer Sat, 17 Aug 2019 14:02:42 +0200 + golang-golang-x-net-dev (1:0.0+git20190724.ca1201d+dfsg-1) unstable; urgency=medium [ Drew Parsons ] diff -Nru golang-golang-x-net-dev-0.0+git20190724.ca1201d+dfsg/http2/server.go golang-golang-x-net-dev-0.0+git20190811.74dc4d7+dfsg/http2/server.go --- golang-golang-x-net-dev-0.0+git20190724.ca1201d+dfsg/http2/server.go 2019-07-24 01:30:45.000000000 +0000 +++ golang-golang-x-net-dev-0.0+git20190811.74dc4d7+dfsg/http2/server.go 2019-08-17 11:56:26.000000000 +0000 @@ -52,10 +52,11 @@ ) const ( - prefaceTimeout = 10 * time.Second - firstSettingsTimeout = 2 * time.Second // should be in-flight with preface anyway - handlerChunkWriteSize = 4 << 10 - defaultMaxStreams = 250 // TODO: make this 100 as the GFE seems to? + prefaceTimeout = 10 * time.Second + firstSettingsTimeout = 2 * time.Second // should be in-flight with preface anyway + handlerChunkWriteSize = 4 << 10 + defaultMaxStreams = 250 // TODO: make this 100 as the GFE seems to? + maxQueuedControlFrames = 10000 ) var ( @@ -163,6 +164,15 @@ return defaultMaxStreams } +// 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 *Server) maxQueuedControlFrames() int { + // TODO: if anybody asks, add a Server field, and remember to define the + // behavior of negative values. + return maxQueuedControlFrames +} + type serverInternalState struct { mu sync.Mutex activeConns map[*serverConn]struct{} @@ -506,6 +516,7 @@ 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 @@ -894,6 +905,14 @@ } } + // 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. @@ -1093,6 +1112,14 @@ } 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() @@ -1210,10 +1237,8 @@ // 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. @@ -1241,6 +1266,9 @@ } if !sc.inGoAway || sc.goAwayCode == ErrCodeNo { if wr, ok := sc.writeSched.Pop(); ok { + if wr.isControl() { + sc.queuedControlFrames-- + } sc.startFrameWrite(wr) continue } @@ -1533,6 +1561,8 @@ 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 diff -Nru golang-golang-x-net-dev-0.0+git20190724.ca1201d+dfsg/http2/server_test.go golang-golang-x-net-dev-0.0+git20190811.74dc4d7+dfsg/http2/server_test.go --- golang-golang-x-net-dev-0.0+git20190724.ca1201d+dfsg/http2/server_test.go 2019-07-24 01:30:45.000000000 +0000 +++ golang-golang-x-net-dev-0.0+git20190811.74dc4d7+dfsg/http2/server_test.go 2019-08-17 11:56:26.000000000 +0000 @@ -1160,6 +1160,32 @@ } } +func TestServer_MaxQueuedControlFrames(t *testing.T) { + if testing.Short() { + t.Skip("skipping in short mode") + } + + st := newServerTester(t, nil) + defer st.Close() + st.greet() + + const extraPings = 500000 // enough to fill the TCP buffers + + for i := 0; i < maxQueuedControlFrames+extraPings; i++ { + pingData := [8]byte{1, 2, 3, 4, 5, 6, 7, 8} + if err := st.fr.WritePing(false, pingData); err != nil { + if i == 0 { + t.Fatal(err) + } + // We expect the connection to get closed by the server when the TCP + // buffer fills up and the write queue reaches MaxQueuedControlFrames. + t.Logf("sent %d PING frames", i) + return + } + } + t.Errorf("unexpected success sending all PING frames") +} + func TestServer_RejectsLargeFrames(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("see golang.org/issue/13434") diff -Nru golang-golang-x-net-dev-0.0+git20190724.ca1201d+dfsg/http2/writesched.go golang-golang-x-net-dev-0.0+git20190811.74dc4d7+dfsg/http2/writesched.go --- golang-golang-x-net-dev-0.0+git20190724.ca1201d+dfsg/http2/writesched.go 2019-07-24 01:30:45.000000000 +0000 +++ golang-golang-x-net-dev-0.0+git20190811.74dc4d7+dfsg/http2/writesched.go 2019-08-17 11:56:26.000000000 +0000 @@ -32,7 +32,7 @@ // 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 FrameWriteRequest, ok bool) } @@ -76,6 +76,12 @@ 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 FrameWriteRequest) 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 FrameWriteRequest) DataSize() int {