reject invalid active_connection_id_limit transport parameter values (#3687)

fuzzing/handshake/cmd/corpus.go

@@ -78,7 +78,7 @@ func main() {
 		protocol.ConnectionID{},
 		nil,
 		nil,
-		&wire.TransportParameters{},
+		&wire.TransportParameters{ActiveConnectionIDLimit: 2},
 		runner,
 		&tls.Config{
 			ServerName:         "localhost",
@@ -102,7 +102,7 @@ func main() {
 		protocol.ConnectionID{},
 		nil,
 		nil,
-		&wire.TransportParameters{},
+		&wire.TransportParameters{ActiveConnectionIDLimit: 2},
 		runner,
 		config,
 		nil,

fuzzing/transportparameters/cmd/corpus.go

@@ -38,7 +38,7 @@ func main() {
 			MaxUniStreamNum:                protocol.StreamNum(getRandomValue()),
 			MaxBidiStreamNum:               protocol.StreamNum(getRandomValue()),
 			MaxIdleTimeout:                 time.Duration(getRandomValue()),
-			ActiveConnectionIDLimit:        getRandomValue(),
+			ActiveConnectionIDLimit:        getRandomValue() + 2,
 		}
 		if rand.Int()%2 == 0 {
 			tp.OriginalDestinationConnectionID = protocol.ParseConnectionID(getRandomData(rand.Intn(21)))

internal/handshake/crypto_setup_test.go

@@ -428,7 +428,7 @@ var _ = Describe("Crypto Setup TLS", func() {
 			_, _, clientErr, _, serverErr := handshakeWithTLSConf(
 				clientConf, serverConf,
 				&utils.RTTStats{}, &utils.RTTStats{},
-				&wire.TransportParameters{}, &wire.TransportParameters{},
+				&wire.TransportParameters{ActiveConnectionIDLimit: 2}, &wire.TransportParameters{ActiveConnectionIDLimit: 2},
 				false,
 			)
 			Expect(clientErr).ToNot(HaveOccurred())
@@ -440,7 +440,7 @@ var _ = Describe("Crypto Setup TLS", func() {
 			_, _, clientErr, _, serverErr := handshakeWithTLSConf(
 				clientConf, serverConf,
 				&utils.RTTStats{}, &utils.RTTStats{},
-				&wire.TransportParameters{}, &wire.TransportParameters{},
+				&wire.TransportParameters{ActiveConnectionIDLimit: 2}, &wire.TransportParameters{ActiveConnectionIDLimit: 2},
 				false,
 			)
 			Expect(clientErr).ToNot(HaveOccurred())
@@ -453,7 +453,7 @@ var _ = Describe("Crypto Setup TLS", func() {
 			_, _, clientErr, _, serverErr := handshakeWithTLSConf(
 				clientConf, serverConf,
 				&utils.RTTStats{}, &utils.RTTStats{},
-				&wire.TransportParameters{}, &wire.TransportParameters{},
+				&wire.TransportParameters{ActiveConnectionIDLimit: 2}, &wire.TransportParameters{ActiveConnectionIDLimit: 2},
 				false,
 			)
 			Expect(clientErr).ToNot(HaveOccurred())
@@ -502,7 +502,7 @@ var _ = Describe("Crypto Setup TLS", func() {
 		It("receives transport parameters", func() {
 			var cTransportParametersRcvd, sTransportParametersRcvd *wire.TransportParameters
 			cChunkChan, cInitialStream, cHandshakeStream := initStreams()
-			cTransportParameters := &wire.TransportParameters{MaxIdleTimeout: 0x42 * time.Second}
+			cTransportParameters := &wire.TransportParameters{ActiveConnectionIDLimit: 2, MaxIdleTimeout: 0x42 * time.Second}
 			cRunner := NewMockHandshakeRunner(mockCtrl)
 			cRunner.EXPECT().OnReceivedParams(gomock.Any()).Do(func(tp *wire.TransportParameters) { sTransportParametersRcvd = tp })
 			cRunner.EXPECT().OnHandshakeComplete()
@@ -528,8 +528,9 @@ var _ = Describe("Crypto Setup TLS", func() {
 			sRunner.EXPECT().OnReceivedParams(gomock.Any()).Do(func(tp *wire.TransportParameters) { cTransportParametersRcvd = tp })
 			sRunner.EXPECT().OnHandshakeComplete()
 			sTransportParameters := &wire.TransportParameters{
-				MaxIdleTimeout:      0x1337 * time.Second,
-				StatelessResetToken: &token,
+				MaxIdleTimeout:          0x1337 * time.Second,
+				StatelessResetToken:     &token,
+				ActiveConnectionIDLimit: 2,
 			}
 			server := NewCryptoSetupServer(
 				sInitialStream,
@@ -571,7 +572,7 @@ var _ = Describe("Crypto Setup TLS", func() {
 					protocol.ConnectionID{},
 					nil,
 					nil,
-					&wire.TransportParameters{},
+					&wire.TransportParameters{ActiveConnectionIDLimit: 2},
 					cRunner,
 					clientConf,
 					false,
@@ -592,7 +593,7 @@ var _ = Describe("Crypto Setup TLS", func() {
 					protocol.ConnectionID{},
 					nil,
 					nil,
-					&wire.TransportParameters{StatelessResetToken: &token},
+					&wire.TransportParameters{ActiveConnectionIDLimit: 2, StatelessResetToken: &token},
 					sRunner,
 					serverConf,
 					nil,
@@ -630,7 +631,7 @@ var _ = Describe("Crypto Setup TLS", func() {
 					protocol.ConnectionID{},
 					nil,
 					nil,
-					&wire.TransportParameters{},
+					&wire.TransportParameters{ActiveConnectionIDLimit: 2},
 					cRunner,
 					clientConf,
 					false,
@@ -651,7 +652,7 @@ var _ = Describe("Crypto Setup TLS", func() {
 					protocol.ConnectionID{},
 					nil,
 					nil,
-					&wire.TransportParameters{StatelessResetToken: &token},
+					&wire.TransportParameters{ActiveConnectionIDLimit: 2, StatelessResetToken: &token},
 					sRunner,
 					serverConf,
 					nil,
@@ -693,7 +694,7 @@ var _ = Describe("Crypto Setup TLS", func() {
 				clientHelloWrittenChan, client, clientErr, server, serverErr := handshakeWithTLSConf(
 					clientConf, serverConf,
 					clientOrigRTTStats, &utils.RTTStats{},
-					&wire.TransportParameters{}, &wire.TransportParameters{},
+					&wire.TransportParameters{ActiveConnectionIDLimit: 2}, &wire.TransportParameters{ActiveConnectionIDLimit: 2},
 					false,
 				)
 				Expect(clientErr).ToNot(HaveOccurred())
@@ -709,7 +710,7 @@ var _ = Describe("Crypto Setup TLS", func() {
 				clientHelloWrittenChan, client, clientErr, server, serverErr = handshakeWithTLSConf(
 					clientConf, serverConf,
 					clientRTTStats, &utils.RTTStats{},
-					&wire.TransportParameters{}, &wire.TransportParameters{},
+					&wire.TransportParameters{ActiveConnectionIDLimit: 2}, &wire.TransportParameters{ActiveConnectionIDLimit: 2},
 					false,
 				)
 				Expect(clientErr).ToNot(HaveOccurred())
@@ -734,7 +735,7 @@ var _ = Describe("Crypto Setup TLS", func() {
 				_, client, clientErr, server, serverErr := handshakeWithTLSConf(
 					clientConf, serverConf,
 					&utils.RTTStats{}, &utils.RTTStats{},
-					&wire.TransportParameters{}, &wire.TransportParameters{},
+					&wire.TransportParameters{ActiveConnectionIDLimit: 2}, &wire.TransportParameters{ActiveConnectionIDLimit: 2},
 					false,
 				)
 				Expect(clientErr).ToNot(HaveOccurred())
@@ -748,7 +749,7 @@ var _ = Describe("Crypto Setup TLS", func() {
 				_, client, clientErr, server, serverErr = handshakeWithTLSConf(
 					clientConf, serverConf,
 					&utils.RTTStats{}, &utils.RTTStats{},
-					&wire.TransportParameters{}, &wire.TransportParameters{},
+					&wire.TransportParameters{ActiveConnectionIDLimit: 2}, &wire.TransportParameters{ActiveConnectionIDLimit: 2},
 					false,
 				)
 				Expect(clientErr).ToNot(HaveOccurred())
@@ -776,7 +777,8 @@ var _ = Describe("Crypto Setup TLS", func() {
 				clientHelloWrittenChan, client, clientErr, server, serverErr := handshakeWithTLSConf(
 					clientConf, serverConf,
 					clientOrigRTTStats, serverOrigRTTStats,
-					&wire.TransportParameters{}, &wire.TransportParameters{InitialMaxData: initialMaxData},
+					&wire.TransportParameters{ActiveConnectionIDLimit: 2},
+					&wire.TransportParameters{ActiveConnectionIDLimit: 2, InitialMaxData: initialMaxData},
 					true,
 				)
 				Expect(clientErr).ToNot(HaveOccurred())
@@ -795,7 +797,8 @@ var _ = Describe("Crypto Setup TLS", func() {
 				clientHelloWrittenChan, client, clientErr, server, serverErr = handshakeWithTLSConf(
 					clientConf, serverConf,
 					clientRTTStats, serverRTTStats,
-					&wire.TransportParameters{}, &wire.TransportParameters{InitialMaxData: initialMaxData},
+					&wire.TransportParameters{ActiveConnectionIDLimit: 2},
+					&wire.TransportParameters{ActiveConnectionIDLimit: 2, InitialMaxData: initialMaxData},
 					true,
 				)
 				Expect(clientErr).ToNot(HaveOccurred())
@@ -829,7 +832,8 @@ var _ = Describe("Crypto Setup TLS", func() {
 				clientHelloWrittenChan, client, clientErr, server, serverErr := handshakeWithTLSConf(
 					clientConf, serverConf,
 					clientOrigRTTStats, &utils.RTTStats{},
-					&wire.TransportParameters{}, &wire.TransportParameters{InitialMaxData: initialMaxData},
+					&wire.TransportParameters{ActiveConnectionIDLimit: 2},
+					&wire.TransportParameters{ActiveConnectionIDLimit: 2, InitialMaxData: initialMaxData},
 					true,
 				)
 				Expect(clientErr).ToNot(HaveOccurred())
@@ -847,7 +851,8 @@ var _ = Describe("Crypto Setup TLS", func() {
 				clientHelloWrittenChan, client, clientErr, server, serverErr = handshakeWithTLSConf(
 					clientConf, serverConf,
 					clientRTTStats, &utils.RTTStats{},
-					&wire.TransportParameters{}, &wire.TransportParameters{InitialMaxData: initialMaxData - 1},
+					&wire.TransportParameters{ActiveConnectionIDLimit: 2},
+					&wire.TransportParameters{ActiveConnectionIDLimit: 2, InitialMaxData: initialMaxData - 1},
 					true,
 				)
 				Expect(clientErr).ToNot(HaveOccurred())

internal/handshake/session_ticket_test.go

@@ -17,6 +17,7 @@ var _ = Describe("Session Ticket", func() {
 			Parameters: &wire.TransportParameters{
 				InitialMaxStreamDataBidiLocal:  1,
 				InitialMaxStreamDataBidiRemote: 2,
+				ActiveConnectionIDLimit:        10,
 			},
 			RTT: 1337 * time.Microsecond,
 		}
@@ -24,6 +25,7 @@ var _ = Describe("Session Ticket", func() {
 		Expect(t.Unmarshal(ticket.Marshal())).To(Succeed())
 		Expect(t.Parameters.InitialMaxStreamDataBidiLocal).To(BeEquivalentTo(1))
 		Expect(t.Parameters.InitialMaxStreamDataBidiRemote).To(BeEquivalentTo(2))
+		Expect(t.Parameters.ActiveConnectionIDLimit).To(BeEquivalentTo(10))
 		Expect(t.RTT).To(Equal(1337 * time.Microsecond))
 	})
 

internal/wire/transport_parameter_test.go

@@ -100,7 +100,7 @@ var _ = Describe("Transport Parameters", func() {
 			RetrySourceConnectionID:         &rcid,
 			AckDelayExponent:                13,
 			MaxAckDelay:                     42 * time.Millisecond,
-			ActiveConnectionIDLimit:         getRandomValue(),
+			ActiveConnectionIDLimit:         2 + getRandomValueUpTo(math.MaxInt64-2),
 			MaxDatagramFrameSize:            protocol.ByteCount(getRandomValue()),
 		}
 		data := params.Marshal(protocol.PerspectiveServer)
@@ -127,7 +127,8 @@ var _ = Describe("Transport Parameters", func() {
 
 	It("doesn't marshal a retry_source_connection_id, if no Retry was performed", func() {
 		data := (&TransportParameters{
-			StatelessResetToken: &protocol.StatelessResetToken{},
+			StatelessResetToken:     &protocol.StatelessResetToken{},
+			ActiveConnectionIDLimit: 2,
 		}).Marshal(protocol.PerspectiveServer)
 		p := &TransportParameters{}
 		Expect(p.Unmarshal(data, protocol.PerspectiveServer)).To(Succeed())
@@ -139,6 +140,7 @@ var _ = Describe("Transport Parameters", func() {
 		data := (&TransportParameters{
 			RetrySourceConnectionID: &rcid,
 			StatelessResetToken:     &protocol.StatelessResetToken{},
+			ActiveConnectionIDLimit: 2,
 		}).Marshal(protocol.PerspectiveServer)
 		p := &TransportParameters{}
 		Expect(p.Unmarshal(data, protocol.PerspectiveServer)).To(Succeed())
@@ -231,6 +233,18 @@ var _ = Describe("Transport Parameters", func() {
 		Expect(float32(dataLen) / num).To(BeNumerically("~", float32(defaultLen)/num+float32(entryLen), 1))
 	})
 
+	It("errors when the active_connection_id_limit is too small", func() {
+		data := (&TransportParameters{
+			ActiveConnectionIDLimit: 1,
+			StatelessResetToken:     &protocol.StatelessResetToken{},
+		}).Marshal(protocol.PerspectiveServer)
+		p := &TransportParameters{}
+		Expect(p.Unmarshal(data, protocol.PerspectiveServer)).To(MatchError(&qerr.TransportError{
+			ErrorCode:    qerr.TransportParameterError,
+			ErrorMessage: "invalid value for active_connection_id_limit: 1 (minimum 2)",
+		}))
+	})
+
 	It("errors when the ack_delay_exponenent is too large", func() {
 		data := (&TransportParameters{
 			AckDelayExponent:    21,
@@ -265,8 +279,9 @@ var _ = Describe("Transport Parameters", func() {
 
 	It("sets the default value for the ack_delay_exponent, when no value was sent", func() {
 		data := (&TransportParameters{
-			AckDelayExponent:    protocol.DefaultAckDelayExponent,
-			StatelessResetToken: &protocol.StatelessResetToken{},
+			AckDelayExponent:        protocol.DefaultAckDelayExponent,
+			StatelessResetToken:     &protocol.StatelessResetToken{},
+			ActiveConnectionIDLimit: 2,
 		}).Marshal(protocol.PerspectiveServer)
 		p := &TransportParameters{}
 		Expect(p.Unmarshal(data, protocol.PerspectiveServer)).To(Succeed())
@@ -416,8 +431,9 @@ var _ = Describe("Transport Parameters", func() {
 
 		It("marshals and unmarshals", func() {
 			data := (&TransportParameters{
-				PreferredAddress:    pa,
-				StatelessResetToken: &protocol.StatelessResetToken{},
+				PreferredAddress:        pa,
+				StatelessResetToken:     &protocol.StatelessResetToken{},
+				ActiveConnectionIDLimit: 2,
 			}).Marshal(protocol.PerspectiveServer)
 			p := &TransportParameters{}
 			Expect(p.Unmarshal(data, protocol.PerspectiveServer)).To(Succeed())
@@ -483,7 +499,7 @@ var _ = Describe("Transport Parameters", func() {
 				InitialMaxData:                 protocol.ByteCount(getRandomValue()),
 				MaxBidiStreamNum:               protocol.StreamNum(getRandomValueUpTo(int64(protocol.MaxStreamCount))),
 				MaxUniStreamNum:                protocol.StreamNum(getRandomValueUpTo(int64(protocol.MaxStreamCount))),
-				ActiveConnectionIDLimit:        getRandomValue(),
+				ActiveConnectionIDLimit:        2 + getRandomValueUpTo(math.MaxInt64-2),
 			}
 			Expect(params.ValidFor0RTT(params)).To(BeTrue())
 			b := params.MarshalForSessionTicket(nil)

internal/wire/transport_parameters.go

@@ -303,6 +303,9 @@ func (p *TransportParameters) readNumericTransportParameter(
 		}
 		p.MaxAckDelay = time.Duration(val) * time.Millisecond
 	case activeConnectionIDLimitParameterID:
+		if val < 2 {
+			return fmt.Errorf("invalid value for active_connection_id_limit: %d (minimum 2)", val)
+		}
 		p.ActiveConnectionIDLimit = val
 	case maxDatagramFrameSizeParameterID:
 		p.MaxDatagramFrameSize = protocol.ByteCount(val)