diff --git a/Changelog.md b/Changelog.md index 02e5593e..bb8559cf 100644 --- a/Changelog.md +++ b/Changelog.md @@ -8,6 +8,7 @@ - Add a `quic.Config` option to request truncation of the connection ID from a server - Add a `quic.Config` option to configure the source address validation - Add a `quic.Config` option to configure the handshake timeout +- Add a `quic.Config` option to configure the idle timeout - Add a `quic.Config` option to configure keep-alive - Implement `net.Conn`-style deadlines for streams - Remove the `tls.Config` from the `quic.Config`. The `tls.Config` must now be passed to the `Dial` and `Listen` functions as a separate parameter. See the [Godoc](https://godoc.org/github.com/lucas-clemente/quic-go) for details. diff --git a/client.go b/client.go index f076745c..081f1259 100644 --- a/client.go +++ b/client.go @@ -153,6 +153,10 @@ func populateClientConfig(config *Config) *Config { if config.HandshakeTimeout != 0 { handshakeTimeout = config.HandshakeTimeout } + idleTimeout := protocol.DefaultIdleTimeout + if config.IdleTimeout != 0 { + idleTimeout = config.IdleTimeout + } maxReceiveStreamFlowControlWindow := config.MaxReceiveStreamFlowControlWindow if maxReceiveStreamFlowControlWindow == 0 { @@ -166,6 +170,7 @@ func populateClientConfig(config *Config) *Config { return &Config{ Versions: versions, HandshakeTimeout: handshakeTimeout, + IdleTimeout: idleTimeout, RequestConnectionIDTruncation: config.RequestConnectionIDTruncation, MaxReceiveStreamFlowControlWindow: maxReceiveStreamFlowControlWindow, MaxReceiveConnectionFlowControlWindow: maxReceiveConnectionFlowControlWindow, diff --git a/client_test.go b/client_test.go index d65bc901..fc0271fb 100644 --- a/client_test.go +++ b/client_test.go @@ -184,10 +184,12 @@ var _ = Describe("Client", func() { It("setups with the right values", func() { config := &Config{ HandshakeTimeout: 1337 * time.Minute, + IdleTimeout: 42 * time.Hour, RequestConnectionIDTruncation: true, } c := populateClientConfig(config) Expect(c.HandshakeTimeout).To(Equal(1337 * time.Minute)) + Expect(c.IdleTimeout).To(Equal(42 * time.Hour)) Expect(c.RequestConnectionIDTruncation).To(BeTrue()) }) @@ -195,6 +197,7 @@ var _ = Describe("Client", func() { c := populateClientConfig(&Config{}) Expect(c.Versions).To(Equal(protocol.SupportedVersions)) Expect(c.HandshakeTimeout).To(Equal(protocol.DefaultHandshakeTimeout)) + Expect(c.IdleTimeout).To(Equal(protocol.DefaultIdleTimeout)) Expect(c.RequestConnectionIDTruncation).To(BeFalse()) }) diff --git a/handshake/connection_parameters_manager.go b/handshake/connection_parameters_manager.go index bb4a641e..81dadc74 100644 --- a/handshake/connection_parameters_manager.go +++ b/handshake/connection_parameters_manager.go @@ -64,8 +64,11 @@ var ( // NewConnectionParamatersManager creates a new connection parameters manager func NewConnectionParamatersManager( - pers protocol.Perspective, v protocol.VersionNumber, - maxReceiveStreamFlowControlWindow protocol.ByteCount, maxReceiveConnectionFlowControlWindow protocol.ByteCount, + pers protocol.Perspective, + v protocol.VersionNumber, + maxReceiveStreamFlowControlWindow protocol.ByteCount, + maxReceiveConnectionFlowControlWindow protocol.ByteCount, + idleTimeout time.Duration, ) ConnectionParametersManager { h := &connectionParametersManager{ perspective: pers, @@ -78,12 +81,11 @@ func NewConnectionParamatersManager( maxReceiveConnectionFlowControlWindow: maxReceiveConnectionFlowControlWindow, } + h.idleConnectionStateLifetime = idleTimeout if h.perspective == protocol.PerspectiveServer { - h.idleConnectionStateLifetime = protocol.DefaultIdleTimeout h.maxStreamsPerConnection = protocol.MaxStreamsPerConnection // this is the value negotiated based on what the client sent h.maxIncomingDynamicStreamsPerConnection = protocol.MaxStreamsPerConnection // "incoming" seen from the client's perspective } else { - h.idleConnectionStateLifetime = protocol.MaxIdleTimeoutClient h.maxStreamsPerConnection = protocol.MaxStreamsPerConnection // this is the value negotiated based on what the client sent h.maxIncomingDynamicStreamsPerConnection = protocol.MaxStreamsPerConnection // "incoming" seen from the server's perspective } @@ -163,10 +165,7 @@ func (h *connectionParametersManager) negotiateMaxIncomingDynamicStreamsPerConne } func (h *connectionParametersManager) negotiateIdleConnectionStateLifetime(clientValue time.Duration) time.Duration { - if h.perspective == protocol.PerspectiveServer { - return utils.MinDuration(clientValue, protocol.MaxIdleTimeoutServer) - } - return utils.MinDuration(clientValue, protocol.MaxIdleTimeoutClient) + return utils.MinDuration(clientValue, h.idleConnectionStateLifetime) } // GetHelloMap gets all parameters needed for the Hello message diff --git a/handshake/connection_parameters_manager_test.go b/handshake/connection_parameters_manager_test.go index 99425143..d290262e 100644 --- a/handshake/connection_parameters_manager_test.go +++ b/handshake/connection_parameters_manager_test.go @@ -18,12 +18,21 @@ var _ = Describe("ConnectionsParameterManager", func() { maxReceiveConnectionFlowControlWindowServer := protocol.ByteCount(math.Floor(1.5 * MB)) // default is 1.5 MB maxReceiveStreamFlowControlWindowClient := protocol.ByteCount(math.Floor(6.4 * MB)) // default is 6 MB maxReceiveConnectionFlowControlWindowClient := protocol.ByteCount(math.Floor(13 * MB)) // default is 15 MB + idleTimeout := 42 * time.Second BeforeEach(func() { - cpm = NewConnectionParamatersManager(protocol.PerspectiveServer, protocol.Version36, - maxReceiveStreamFlowControlWindowServer, maxReceiveConnectionFlowControlWindowServer, + cpm = NewConnectionParamatersManager( + protocol.PerspectiveServer, + protocol.Version36, + maxReceiveStreamFlowControlWindowServer, + maxReceiveConnectionFlowControlWindowServer, + idleTimeout, ).(*connectionParametersManager) - cpmClient = NewConnectionParamatersManager(protocol.PerspectiveClient, protocol.Version36, - maxReceiveStreamFlowControlWindowClient, maxReceiveConnectionFlowControlWindowClient, + cpmClient = NewConnectionParamatersManager( + protocol.PerspectiveClient, + protocol.Version36, + maxReceiveStreamFlowControlWindowClient, + maxReceiveConnectionFlowControlWindowClient, + idleTimeout, ).(*connectionParametersManager) }) @@ -94,7 +103,7 @@ var _ = Describe("ConnectionsParameterManager", func() { entryMap, err := cpmClient.GetHelloMap() Expect(err).ToNot(HaveOccurred()) Expect(entryMap).To(HaveKey(TagICSL)) - Expect(binary.LittleEndian.Uint32(entryMap[TagICSL])).To(BeEquivalentTo(protocol.MaxIdleTimeoutClient / time.Second)) + Expect(binary.LittleEndian.Uint32(entryMap[TagICSL])).To(BeEquivalentTo(idleTimeout / time.Second)) Expect(entryMap).To(HaveKey(TagMSPC)) Expect(binary.LittleEndian.Uint32(entryMap[TagMSPC])).To(BeEquivalentTo(protocol.MaxStreamsPerConnection)) Expect(entryMap).To(HaveKey(TagMIDS)) @@ -200,17 +209,15 @@ var _ = Describe("ConnectionsParameterManager", func() { Context("idle connection state lifetime", func() { It("has initial idle connection state lifetime", func() { - Expect(cpm.GetIdleConnectionStateLifetime()).To(Equal(protocol.DefaultIdleTimeout)) + Expect(cpm.GetIdleConnectionStateLifetime()).To(Equal(idleTimeout)) }) It("negotiates correctly when the peer wants a longer lifetime", func() { - Expect(cpm.negotiateIdleConnectionStateLifetime(protocol.MaxIdleTimeoutServer + 10*time.Second)).To(Equal(protocol.MaxIdleTimeoutServer)) - Expect(cpmClient.negotiateIdleConnectionStateLifetime(protocol.MaxIdleTimeoutClient + 10*time.Second)).To(Equal(protocol.MaxIdleTimeoutClient)) + Expect(cpm.negotiateIdleConnectionStateLifetime(idleTimeout + 10*time.Second)).To(Equal(idleTimeout)) }) It("negotiates correctly when the peer wants a shorter lifetime", func() { - Expect(cpm.negotiateIdleConnectionStateLifetime(protocol.MaxIdleTimeoutServer - 1*time.Second)).To(Equal(protocol.MaxIdleTimeoutServer - 1*time.Second)) - Expect(cpmClient.negotiateIdleConnectionStateLifetime(protocol.MaxIdleTimeoutClient - 1*time.Second)).To(Equal(protocol.MaxIdleTimeoutClient - 1*time.Second)) + Expect(cpm.negotiateIdleConnectionStateLifetime(idleTimeout - 3*time.Second)).To(Equal(idleTimeout - 3*time.Second)) }) It("sets the negotiated lifetime", func() { @@ -229,7 +236,7 @@ var _ = Describe("ConnectionsParameterManager", func() { } err := cpm.SetFromMap(values) Expect(err).To(MatchError(ErrMalformedTag)) - Expect(cpm.GetIdleConnectionStateLifetime()).To(Equal(protocol.DefaultIdleTimeout)) + Expect(cpm.GetIdleConnectionStateLifetime()).To(Equal(idleTimeout)) }) It("gets idle connection state lifetime", func() { diff --git a/handshake/crypto_setup_client_test.go b/handshake/crypto_setup_client_test.go index 2f66e0ba..c689a8c4 100644 --- a/handshake/crypto_setup_client_test.go +++ b/handshake/crypto_setup_client_test.go @@ -111,8 +111,11 @@ var _ = Describe("Client Crypto Setup", func() { version, stream, nil, - NewConnectionParamatersManager(protocol.PerspectiveClient, version, + NewConnectionParamatersManager( + protocol.PerspectiveClient, + version, protocol.DefaultMaxReceiveStreamFlowControlWindowClient, protocol.DefaultMaxReceiveConnectionFlowControlWindowClient, + protocol.DefaultIdleTimeout, ), aeadChanged, &TransportParameters{}, diff --git a/handshake/crypto_setup_server_test.go b/handshake/crypto_setup_server_test.go index 2fd41175..b73d5e03 100644 --- a/handshake/crypto_setup_server_test.go +++ b/handshake/crypto_setup_server_test.go @@ -184,8 +184,11 @@ var _ = Describe("Server Crypto Setup", func() { Expect(err).NotTo(HaveOccurred()) version = protocol.SupportedVersions[len(protocol.SupportedVersions)-1] supportedVersions = []protocol.VersionNumber{version, 98, 99} - cpm = NewConnectionParamatersManager(protocol.PerspectiveServer, protocol.VersionWhatever, + cpm = NewConnectionParamatersManager( + protocol.PerspectiveServer, + protocol.VersionWhatever, protocol.DefaultMaxReceiveStreamFlowControlWindowServer, protocol.DefaultMaxReceiveConnectionFlowControlWindowServer, + protocol.DefaultIdleTimeout, ) csInt, err := NewCryptoSetup( protocol.ConnectionID(42), diff --git a/interface.go b/interface.go index 41b607ca..771d98dd 100644 --- a/interface.go +++ b/interface.go @@ -100,6 +100,11 @@ type Config struct { // If the timeout is exceeded, the connection is closed. // If this value is zero, the timeout is set to 10 seconds. HandshakeTimeout time.Duration + // IdleTimeout is the maximum duration that may pass without any incoming network activity. + // This value only applies after the handshake has completed. + // If the timeout is exceeded, the connection is closed. + // If this value is zero, the timeout is set to 30 seconds. + IdleTimeout time.Duration // AcceptSTK determines if an STK is accepted. // It is called with stk = nil if the client didn't send an STK. // If not set, it verifies that the address matches, and that the STK was issued within the last 24 hours. diff --git a/protocol/server_parameters.go b/protocol/server_parameters.go index c2ab602b..2b0c3872 100644 --- a/protocol/server_parameters.go +++ b/protocol/server_parameters.go @@ -119,18 +119,9 @@ const CryptoParameterMaxLength = 4000 // EphermalKeyLifetime is the lifetime of the ephermal key during the handshake, see handshake.getEphermalKEX. const EphermalKeyLifetime = time.Minute -// InitialIdleTimeout is the timeout before the handshake succeeds. -const InitialIdleTimeout = 5 * time.Second - -// DefaultIdleTimeout is the default idle timeout, for the server +// DefaultIdleTimeout is the default idle timeout const DefaultIdleTimeout = 30 * time.Second -// MaxIdleTimeoutServer is the maximum idle timeout that can be negotiated, for the server -const MaxIdleTimeoutServer = 1 * time.Minute - -// MaxIdleTimeoutClient is the idle timeout that the client suggests to the server -const MaxIdleTimeoutClient = 2 * time.Minute - // DefaultHandshakeTimeout is the default timeout for a connection until the crypto handshake succeeds. const DefaultHandshakeTimeout = 10 * time.Second diff --git a/server.go b/server.go index 74cde7a6..b45b760b 100644 --- a/server.go +++ b/server.go @@ -129,6 +129,10 @@ func populateServerConfig(config *Config) *Config { if config.HandshakeTimeout != 0 { handshakeTimeout = config.HandshakeTimeout } + idleTimeout := protocol.DefaultIdleTimeout + if config.IdleTimeout != 0 { + idleTimeout = config.IdleTimeout + } maxReceiveStreamFlowControlWindow := config.MaxReceiveStreamFlowControlWindow if maxReceiveStreamFlowControlWindow == 0 { @@ -142,6 +146,7 @@ func populateServerConfig(config *Config) *Config { return &Config{ Versions: versions, HandshakeTimeout: handshakeTimeout, + IdleTimeout: idleTimeout, AcceptSTK: vsa, MaxReceiveStreamFlowControlWindow: maxReceiveStreamFlowControlWindow, MaxReceiveConnectionFlowControlWindow: maxReceiveConnectionFlowControlWindow, diff --git a/server_test.go b/server_test.go index 3fa574ce..bc26a2ec 100644 --- a/server_test.go +++ b/server_test.go @@ -355,6 +355,7 @@ var _ = Describe("Server", func() { Versions: supportedVersions, AcceptSTK: acceptSTK, HandshakeTimeout: 1337 * time.Hour, + IdleTimeout: 42 * time.Minute, } ln, err := Listen(conn, &tls.Config{}, &config) Expect(err).ToNot(HaveOccurred()) @@ -364,6 +365,7 @@ var _ = Describe("Server", func() { Expect(server.scfg).ToNot(BeNil()) Expect(server.config.Versions).To(Equal(supportedVersions)) Expect(server.config.HandshakeTimeout).To(Equal(1337 * time.Hour)) + Expect(server.config.IdleTimeout).To(Equal(42 * time.Minute)) Expect(reflect.ValueOf(server.config.AcceptSTK)).To(Equal(reflect.ValueOf(acceptSTK))) }) @@ -373,6 +375,7 @@ var _ = Describe("Server", func() { server := ln.(*server) Expect(server.config.Versions).To(Equal(protocol.SupportedVersions)) Expect(server.config.HandshakeTimeout).To(Equal(protocol.DefaultHandshakeTimeout)) + Expect(server.config.IdleTimeout).To(Equal(protocol.DefaultIdleTimeout)) Expect(reflect.ValueOf(server.config.AcceptSTK)).To(Equal(reflect.ValueOf(defaultAcceptSTK))) }) diff --git a/session.go b/session.go index cf9da4ef..49c17042 100644 --- a/session.go +++ b/session.go @@ -181,8 +181,13 @@ func (s *session) setup( s.sessionCreationTime = now s.rttStats = &congestion.RTTStats{} - s.connectionParameters = handshake.NewConnectionParamatersManager(s.perspective, s.version, - s.config.MaxReceiveStreamFlowControlWindow, s.config.MaxReceiveConnectionFlowControlWindow) + s.connectionParameters = handshake.NewConnectionParamatersManager( + s.perspective, + s.version, + s.config.MaxReceiveStreamFlowControlWindow, + s.config.MaxReceiveConnectionFlowControlWindow, + s.config.IdleTimeout, + ) s.sentPacketHandler = ackhandler.NewSentPacketHandler(s.rttStats) s.flowControlManager = flowcontrol.NewFlowControlManager(s.connectionParameters, s.rttStats) s.receivedPacketHandler = ackhandler.NewReceivedPacketHandler(s.version) @@ -318,12 +323,12 @@ runLoop: if !s.receivedTooManyUndecrytablePacketsTime.IsZero() && s.receivedTooManyUndecrytablePacketsTime.Add(protocol.PublicResetTimeout).Before(now) && len(s.undecryptablePackets) != 0 { s.closeLocal(qerr.Error(qerr.DecryptionFailure, "too many undecryptable packets received")) } - if now.Sub(s.lastNetworkActivityTime) >= s.idleTimeout() { - s.closeLocal(qerr.Error(qerr.NetworkIdleTimeout, "No recent network activity.")) - } if !s.handshakeComplete && now.Sub(s.sessionCreationTime) >= s.config.HandshakeTimeout { s.closeLocal(qerr.Error(qerr.HandshakeTimeout, "Crypto handshake did not complete in time.")) } + if s.handshakeComplete && now.Sub(s.lastNetworkActivityTime) >= s.idleTimeout() { + s.closeLocal(qerr.Error(qerr.NetworkIdleTimeout, "No recent network activity.")) + } s.garbageCollectStreams() } @@ -368,10 +373,7 @@ func (s *session) maybeResetTimer() { } func (s *session) idleTimeout() time.Duration { - if s.handshakeComplete { - return s.connectionParameters.GetIdleConnectionStateLifetime() - } - return protocol.InitialIdleTimeout + return s.connectionParameters.GetIdleConnectionStateLifetime() } func (s *session) handlePacketImpl(p *receivedPacket) error { diff --git a/session_test.go b/session_test.go index dc04cef0..80b11bda 100644 --- a/session_test.go +++ b/session_test.go @@ -1507,6 +1507,7 @@ var _ = Describe("Session", func() { Context("timeouts", func() { It("times out due to no network activity", func(done Done) { + sess.handshakeComplete = true sess.lastNetworkActivityTime = time.Now().Add(-time.Hour) err := sess.run() // Would normally not return Expect(err.(*qerr.QuicError).ErrorCode).To(Equal(qerr.NetworkIdleTimeout)) @@ -1524,18 +1525,22 @@ var _ = Describe("Session", func() { close(done) }) - It("does not use ICSL before handshake", func(done Done) { + It("does not use ICSL before handshake", func() { + defer sess.Close(nil) sess.lastNetworkActivityTime = time.Now().Add(-time.Minute) mockCpm = mocks.NewMockConnectionParametersManager(mockCtrl) mockCpm.EXPECT().GetIdleConnectionStateLifetime().Return(9999 * time.Second).AnyTimes() mockCpm.EXPECT().TruncateConnectionID().Return(false).AnyTimes() sess.connectionParameters = mockCpm sess.packer.connectionParameters = mockCpm - err := sess.run() // Would normally not return - Expect(err.(*qerr.QuicError).ErrorCode).To(Equal(qerr.NetworkIdleTimeout)) - Expect(mconn.written).To(Receive(ContainSubstring("No recent network activity."))) - Expect(sess.Context().Done()).To(BeClosed()) - close(done) + // the handshake timeout is irrelevant here, since it depends on the time the session was created, + // and not on the last network activity + done := make(chan struct{}) + go func() { + _ = sess.run() + close(done) + }() + Consistently(done).ShouldNot(BeClosed()) }) It("uses ICSL after handshake", func(done Done) {