From 9608e8563f9eb6f2d649802d485c1fe1dc93ef46 Mon Sep 17 00:00:00 2001
From: Marten Seemann <martenseemann@gmail.com>
Date: Tue, 14 Aug 2018 18:35:25 +0700
Subject: [PATCH] only accept 3 retries

While the server is allowed to perform multiple Retries, the client
should impose a limit in order to avoid being caught in an endless loop.
---
 client.go                              |  8 +++-
 client_test.go                         | 55 ++++++++++++++++++++++++++
 internal/protocol/server_parameters.go |  3 ++
 3 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/client.go b/client.go
index 88fc6df2..4bf002fc 100644
--- a/client.go
+++ b/client.go
@@ -29,7 +29,8 @@ type client struct {
 
 	packetHandlers packetHandlerManager
 
-	token []byte
+	token      []byte
+	numRetries int
 
 	versionNegotiated                bool // has the server accepted our version
 	receivedVersionNegotiationPacket bool
@@ -495,6 +496,11 @@ func (c *client) handleRetryPacket(hdr *wire.Header) {
 		c.logger.Debugf("Received spoofed Retry. Original Destination Connection ID: %s, expected: %s", hdr.OrigDestConnectionID, c.destConnID)
 		return
 	}
+	c.numRetries++
+	if c.numRetries > protocol.MaxRetries {
+		c.session.destroy(qerr.CryptoTooManyRejects)
+		return
+	}
 	c.destConnID = hdr.SrcConnectionID
 	c.token = hdr.Token
 	c.session.destroy(errCloseSessionForRetry)
diff --git a/client_test.go b/client_test.go
index 01f3d54e..8a7067ce 100644
--- a/client_test.go
+++ b/client_test.go
@@ -534,6 +534,61 @@ var _ = Describe("Client", func() {
 				Expect(err).ToNot(HaveOccurred())
 				Expect(sessions).To(BeEmpty())
 			})
+
+			It("only accepts 3 retries", func() {
+				manager := NewMockPacketHandlerManager(mockCtrl)
+				manager.EXPECT().Add(gomock.Any(), gomock.Any()).Do(func(id protocol.ConnectionID, handler packetHandler) {
+					go handler.handlePacket(&receivedPacket{
+						header: &wire.Header{
+							IsLongHeader:         true,
+							Type:                 protocol.PacketTypeRetry,
+							Token:                []byte("foobar"),
+							SrcConnectionID:      connID,
+							DestConnectionID:     id,
+							OrigDestConnectionID: connID,
+							Version:              protocol.VersionTLS,
+						},
+					})
+				}).AnyTimes()
+				manager.EXPECT().Add(gomock.Any(), gomock.Any()).AnyTimes()
+				mockMultiplexer.EXPECT().AddConn(packetConn, gomock.Any()).Return(manager, nil)
+
+				config := &Config{Versions: []protocol.VersionNumber{protocol.VersionTLS}}
+				cl.config = config
+
+				sessions := make(chan quicSession, protocol.MaxRetries+1)
+				for i := 0; i < protocol.MaxRetries+1; i++ {
+					run := make(chan error)
+					sess := NewMockQuicSession(mockCtrl)
+					sess.EXPECT().run().DoAndReturn(func() error {
+						return <-run
+					})
+					sess.EXPECT().destroy(gomock.Any()).Do(func(e error) {
+						run <- e
+					})
+					sessions <- sess
+				}
+
+				newTLSClientSession = func(
+					_ connection,
+					_ sessionRunner,
+					_ []byte,
+					_ protocol.ConnectionID,
+					_ protocol.ConnectionID,
+					_ *Config,
+					_ *mint.Config,
+					_ <-chan handshake.TransportParameters,
+					_ protocol.PacketNumber,
+					_ utils.Logger,
+					_ protocol.VersionNumber,
+				) (quicSession, error) {
+					return <-sessions, nil
+				}
+				_, err := Dial(packetConn, addr, "quic.clemente.io:1337", nil, config)
+				Expect(err).To(HaveOccurred())
+				Expect(err.(qerr.ErrorCode)).To(Equal(qerr.CryptoTooManyRejects))
+				Expect(sessions).To(BeEmpty())
+			})
 		})
 
 		Context("version negotiation", func() {
diff --git a/internal/protocol/server_parameters.go b/internal/protocol/server_parameters.go
index aa92c822..aa61b3d7 100644
--- a/internal/protocol/server_parameters.go
+++ b/internal/protocol/server_parameters.go
@@ -149,3 +149,6 @@ const MinPacingDelay time.Duration = 100 * time.Microsecond
 // DefaultConnectionIDLength is the connection ID length that is used for multiplexed connections
 // if no other value is configured.
 const DefaultConnectionIDLength = 4
+
+// MaxRetries is the maximum number of Retries a client will do before failing the connection.
+const MaxRetries = 3