From 9294652ecc13a0eacbe6d009694be8c00d284489 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 22 Aug 2019 10:23:51 +0700 Subject: [PATCH 1/2] reject http3 requests that exceeded the header size limit --- http3/server.go | 12 +++++++++++- http3/server_test.go | 22 ++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/http3/server.go b/http3/server.go index 13e74066a..e505a443c 100644 --- a/http3/server.go +++ b/http3/server.go @@ -155,6 +155,13 @@ func (s *Server) handleConn(sess quic.Session) { } } +func (s *Server) maxHeaderBytes() uint64 { + if s.Server.MaxHeaderBytes <= 0 { + return http.DefaultMaxHeaderBytes + } + return uint64(s.Server.MaxHeaderBytes) +} + // TODO: improve error handling. // Most (but not all) of the errors occurring here are connection-level erros. func (s *Server) handleRequest(str quic.Stream, decoder *qpack.Decoder) error { @@ -168,7 +175,10 @@ func (s *Server) handleRequest(str quic.Stream, decoder *qpack.Decoder) error { str.CancelWrite(quic.ErrorCode(errorUnexpectedFrame)) return errors.New("expected first frame to be a headers frame") } - // TODO: check length + if hf.Length > s.maxHeaderBytes() { + str.CancelWrite(quic.ErrorCode(errorLimitExceeded)) + return fmt.Errorf("Headers frame too large: %d bytes (max: %d)", hf.Length, s.maxHeaderBytes()) + } headerBlock := make([]byte, hf.Length) if _, err := io.ReadFull(str, headerBlock); err != nil { str.CancelWrite(quic.ErrorCode(errorIncompleteRequest)) diff --git a/http3/server_test.go b/http3/server_test.go index 314201e21..254095531 100644 --- a/http3/server_test.go +++ b/http3/server_test.go @@ -181,6 +181,28 @@ var _ = Describe("Server", func() { Expect(hfs).To(HaveKeyWithValue(":status", []string{"200"})) }) + It("errors when the client sends a too large header frame", func() { + s.Server.MaxHeaderBytes = 42 + s.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + Fail("Handler should not be called.") + }) + + requestData := encodeRequest(exampleGetRequest) + buf := &bytes.Buffer{} + (&dataFrame{Length: 6}).Write(buf) // add a body + buf.Write([]byte("foobar")) + responseBuf := &bytes.Buffer{} + setRequest(append(requestData, buf.Bytes()...)) + str.EXPECT().Write(gomock.Any()).DoAndReturn(func(p []byte) (int, error) { + return responseBuf.Write(p) + }).AnyTimes() + + str.EXPECT().CancelWrite(quic.ErrorCode(errorLimitExceeded)) + err := s.handleRequest(str, qpackDecoder) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("Headers frame too large")) + }) + It("cancels reading when the body of POST request is not read", func() { handlerCalled := make(chan struct{}) s.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { From 363de010ca7c9e39e262df66f15c8de017860d93 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 22 Aug 2019 12:08:02 +0700 Subject: [PATCH 2/2] reject http3 responses that exceeded the header size limit --- http3/client.go | 13 ++++++++++++- http3/client_test.go | 24 +++++++++++++++++++++++- http3/roundtrip.go | 10 +++++++++- 3 files changed, 44 insertions(+), 3 deletions(-) diff --git a/http3/client.go b/http3/client.go index 8744422e5..b297b8b78 100644 --- a/http3/client.go +++ b/http3/client.go @@ -17,6 +17,7 @@ import ( ) const defaultUserAgent = "quic-go HTTP/3" +const defaultMaxResponseHeaderBytes = 10 * 1 << 20 // 10 MB var defaultQuicConfig = &quic.Config{KeepAlive: true} @@ -24,6 +25,7 @@ var dialAddr = quic.DialAddr type roundTripperOpts struct { DisableCompression bool + MaxHeaderBytes int64 } // client is a HTTP3 client doing requests @@ -121,6 +123,13 @@ func (c *client) Close() error { return c.session.Close() } +func (c *client) maxHeaderBytes() uint64 { + if c.opts.MaxHeaderBytes <= 0 { + return defaultMaxResponseHeaderBytes + } + return uint64(c.opts.MaxHeaderBytes) +} + // Roundtrip executes a request and returns a response // TODO: handle request cancelations func (c *client) RoundTrip(req *http.Request) (*http.Response, error) { @@ -160,7 +169,9 @@ func (c *client) RoundTrip(req *http.Request) (*http.Response, error) { if !ok { return nil, errors.New("not a HEADERS frame") } - // TODO: check size + if hf.Length > c.maxHeaderBytes() { + return nil, fmt.Errorf("Headers frame too large: %d bytes (max: %d)", hf.Length, c.maxHeaderBytes()) + } headerBlock := make([]byte, hf.Length) if _, err := io.ReadFull(str, headerBlock); err != nil { return nil, err diff --git a/http3/client_test.go b/http3/client_test.go index f8ccc71c5..1b65d2530 100644 --- a/http3/client_test.go +++ b/http3/client_test.go @@ -31,7 +31,7 @@ var _ = Describe("Client", func() { BeforeEach(func() { origDialAddr = dialAddr hostname := "quic.clemente.io:1337" - client = newClient(hostname, nil, &roundTripperOpts{}, nil, nil) + client = newClient(hostname, nil, &roundTripperOpts{MaxHeaderBytes: 1337}, nil, nil) Expect(client.hostname).To(Equal(hostname)) var err error @@ -275,6 +275,28 @@ var _ = Describe("Client", func() { _, err := client.RoundTrip(request) Expect(err).To(MatchError("test done")) }) + + It("errors when the first frame is not a HEADERS frame", func() { + buf := &bytes.Buffer{} + (&dataFrame{Length: 0x42}).Write(buf) + str.EXPECT().Close().MaxTimes(1) + str.EXPECT().Read(gomock.Any()).DoAndReturn(func(b []byte) (int, error) { + return buf.Read(b) + }).AnyTimes() + _, err := client.RoundTrip(request) + Expect(err).To(MatchError("not a HEADERS frame")) + }) + + It("errors when the first frame is not a HEADERS frame", func() { + buf := &bytes.Buffer{} + (&headersFrame{Length: 1338}).Write(buf) + str.EXPECT().Close().MaxTimes(1) + str.EXPECT().Read(gomock.Any()).DoAndReturn(func(b []byte) (int, error) { + return buf.Read(b) + }).AnyTimes() + _, err := client.RoundTrip(request) + Expect(err).To(MatchError("Headers frame too large: 1338 bytes (max: 1337)")) + }) }) Context("gzip compression", func() { diff --git a/http3/roundtrip.go b/http3/roundtrip.go index 493e2fb3f..003e17d53 100644 --- a/http3/roundtrip.go +++ b/http3/roundtrip.go @@ -46,6 +46,11 @@ type RoundTripper struct { // If Dial is nil, quic.DialAddr will be used. Dial func(network, addr string, tlsCfg *tls.Config, cfg *quic.Config) (quic.Session, error) + // MaxResponseHeaderBytes specifies a limit on how many response bytes are + // allowed in the server's response header. + // Zero means to use a default limit. + MaxResponseHeaderBytes int64 + clients map[string]roundTripCloser } @@ -128,7 +133,10 @@ func (r *RoundTripper) getClient(hostname string, onlyCached bool) (http.RoundTr client = newClient( hostname, r.TLSClientConfig, - &roundTripperOpts{DisableCompression: r.DisableCompression}, + &roundTripperOpts{ + DisableCompression: r.DisableCompression, + MaxHeaderBytes: r.MaxResponseHeaderBytes, + }, r.QuicConfig, r.Dial, )