From ce24f7e46ffdaeb7070d8cac9112e069b5bf3a9a Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Tue, 11 Aug 2020 10:43:01 +0700 Subject: [PATCH 1/2] use an int64 for Config.MaxIncoming{Uni}Streams --- client_test.go | 8 ++++---- config_test.go | 8 ++++---- interface.go | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/client_test.go b/client_test.go index ad339521..d77554a7 100644 --- a/client_test.go +++ b/client_test.go @@ -487,8 +487,8 @@ var _ = Describe("Client", func() { c := populateClientConfig(config, false) Expect(c.HandshakeTimeout).To(Equal(1337 * time.Minute)) Expect(c.MaxIdleTimeout).To(Equal(42 * time.Hour)) - Expect(c.MaxIncomingStreams).To(Equal(1234)) - Expect(c.MaxIncomingUniStreams).To(Equal(4321)) + Expect(c.MaxIncomingStreams).To(BeEquivalentTo(1234)) + Expect(c.MaxIncomingUniStreams).To(BeEquivalentTo(4321)) Expect(c.ConnectionIDLength).To(Equal(13)) Expect(c.StatelessResetKey).To(Equal([]byte("foobar"))) Expect(c.QuicTracer).To(Equal(tracer)) @@ -511,7 +511,7 @@ var _ = Describe("Client", func() { } c := populateClientConfig(config, false) Expect(c.MaxIncomingStreams).To(BeZero()) - Expect(c.MaxIncomingUniStreams).To(Equal(4321)) + Expect(c.MaxIncomingUniStreams).To(BeEquivalentTo(4321)) }) It("disables unidirectional streams", func() { @@ -520,7 +520,7 @@ var _ = Describe("Client", func() { MaxIncomingUniStreams: -1, } c := populateClientConfig(config, false) - Expect(c.MaxIncomingStreams).To(Equal(1234)) + Expect(c.MaxIncomingStreams).To(BeEquivalentTo(1234)) Expect(c.MaxIncomingUniStreams).To(BeZero()) }) diff --git a/config_test.go b/config_test.go index fe1ac984..d96fcf80 100644 --- a/config_test.go +++ b/config_test.go @@ -45,9 +45,9 @@ var _ = Describe("Config", func() { case "MaxReceiveConnectionFlowControlWindow": f.Set(reflect.ValueOf(uint64(10))) case "MaxIncomingStreams": - f.Set(reflect.ValueOf(11)) + f.Set(reflect.ValueOf(int64(11))) case "MaxIncomingUniStreams": - f.Set(reflect.ValueOf(12)) + f.Set(reflect.ValueOf(int64(12))) case "StatelessResetKey": f.Set(reflect.ValueOf([]byte{1, 2, 3, 4})) case "KeepAlive": @@ -114,8 +114,8 @@ var _ = Describe("Config", func() { Expect(c.HandshakeTimeout).To(Equal(protocol.DefaultHandshakeTimeout)) Expect(c.MaxReceiveStreamFlowControlWindow).To(BeEquivalentTo(protocol.DefaultMaxReceiveStreamFlowControlWindow)) Expect(c.MaxReceiveConnectionFlowControlWindow).To(BeEquivalentTo(protocol.DefaultMaxReceiveConnectionFlowControlWindow)) - Expect(c.MaxIncomingStreams).To(Equal(protocol.DefaultMaxIncomingStreams)) - Expect(c.MaxIncomingUniStreams).To(Equal(protocol.DefaultMaxIncomingUniStreams)) + Expect(c.MaxIncomingStreams).To(BeEquivalentTo(protocol.DefaultMaxIncomingStreams)) + Expect(c.MaxIncomingUniStreams).To(BeEquivalentTo(protocol.DefaultMaxIncomingUniStreams)) }) It("populates empty fields with default values, for the server", func() { diff --git a/interface.go b/interface.go index d1034f72..9229b216 100644 --- a/interface.go +++ b/interface.go @@ -244,11 +244,11 @@ type Config struct { // MaxIncomingStreams is the maximum number of concurrent bidirectional streams that a peer is allowed to open. // If not set, it will default to 100. // If set to a negative value, it doesn't allow any bidirectional streams. - MaxIncomingStreams int + MaxIncomingStreams int64 // MaxIncomingUniStreams is the maximum number of concurrent unidirectional streams that a peer is allowed to open. // If not set, it will default to 100. // If set to a negative value, it doesn't allow any unidirectional streams. - MaxIncomingUniStreams int + MaxIncomingUniStreams int64 // The StatelessResetKey is used to generate stateless reset tokens. // If no key is configured, sending of stateless resets is disabled. StatelessResetKey []byte From 69df425318e053ccb7e4dc4d5fb2c5594c7928c0 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Tue, 11 Aug 2020 10:44:24 +0700 Subject: [PATCH 2/2] limit Config.MaxIncoming{Uni}Streams to 2^60 --- client.go | 3 +++ config.go | 19 ++++++++++++++++++- config_test.go | 18 ++++++++++++++++++ interface.go | 2 ++ server.go | 3 +++ 5 files changed, 44 insertions(+), 1 deletion(-) diff --git a/client.go b/client.go index 63db2cdb..e77a788f 100644 --- a/client.go +++ b/client.go @@ -164,6 +164,9 @@ func dialContext( if tlsConf == nil { return nil, errors.New("quic: tls.Config not set") } + if err := validateConfig(config); err != nil { + return nil, err + } config = populateClientConfig(config, createdPacketConn) packetHandlers, err := getMultiplexer().AddConn(pconn, config.ConnectionIDLength, config.StatelessResetKey, config.Tracer) if err != nil { diff --git a/config.go b/config.go index 1790bbb1..a8c308db 100644 --- a/config.go +++ b/config.go @@ -1,6 +1,10 @@ package quic -import "github.com/lucas-clemente/quic-go/internal/protocol" +import ( + "errors" + + "github.com/lucas-clemente/quic-go/internal/protocol" +) // Clone clones a Config func (c *Config) Clone() *Config { @@ -8,6 +12,19 @@ func (c *Config) Clone() *Config { return © } +func validateConfig(config *Config) error { + if config == nil { + return nil + } + if config.MaxIncomingStreams > 1<<60 { + return errors.New("invalid value for Config.MaxIncomingStreams") + } + if config.MaxIncomingUniStreams > 1<<60 { + return errors.New("invalid value for Config.MaxIncomingUniStreams") + } + return nil +} + // populateServerConfig populates fields in the quic.Config with their default values, if none are set // it may be called with nil func populateServerConfig(config *Config) *Config { diff --git a/config_test.go b/config_test.go index d96fcf80..ad9b23c7 100644 --- a/config_test.go +++ b/config_test.go @@ -15,6 +15,24 @@ import ( ) var _ = Describe("Config", func() { + Context("validating", func() { + It("validates a nil config", func() { + Expect(validateConfig(nil)).To(Succeed()) + }) + + It("validates a config with normal values", func() { + Expect(validateConfig(populateServerConfig(&Config{}))).To(Succeed()) + }) + + It("errors on too large values for MaxIncomingStreams", func() { + Expect(validateConfig(&Config{MaxIncomingStreams: 1<<60 + 1})).To(MatchError("invalid value for Config.MaxIncomingStreams")) + }) + + It("errors on too large values for MaxIncomingUniStreams", func() { + Expect(validateConfig(&Config{MaxIncomingUniStreams: 1<<60 + 1})).To(MatchError("invalid value for Config.MaxIncomingUniStreams")) + }) + }) + configWithNonZeroNonFunctionFields := func() *Config { c := &Config{} v := reflect.ValueOf(c).Elem() diff --git a/interface.go b/interface.go index 9229b216..bb825542 100644 --- a/interface.go +++ b/interface.go @@ -242,10 +242,12 @@ type Config struct { // If this value is zero, it will default to 1.5 MB for the server and 15 MB for the client. MaxReceiveConnectionFlowControlWindow uint64 // MaxIncomingStreams is the maximum number of concurrent bidirectional streams that a peer is allowed to open. + // Values above 2^60 are invalid. // If not set, it will default to 100. // If set to a negative value, it doesn't allow any bidirectional streams. MaxIncomingStreams int64 // MaxIncomingUniStreams is the maximum number of concurrent unidirectional streams that a peer is allowed to open. + // Values above 2^60 are invalid. // If not set, it will default to 100. // If set to a negative value, it doesn't allow any unidirectional streams. MaxIncomingUniStreams int64 diff --git a/server.go b/server.go index c4fa4291..6766f2f4 100644 --- a/server.go +++ b/server.go @@ -171,6 +171,9 @@ func listen(conn net.PacketConn, tlsConf *tls.Config, config *Config, acceptEarl if tlsConf == nil { return nil, errors.New("quic: tls.Config not set") } + if err := validateConfig(config); err != nil { + return nil, err + } config = populateServerConfig(config) for _, v := range config.Versions { if !protocol.IsValidVersion(v) {