From 0a22da7e03caf76058b00c6c448e105334fe77e1 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 24 Jul 2025 15:43:34 +0200 Subject: [PATCH] wire: add support for the min_ack_delay transport parameter (#5266) * wire: implement parsing and writing of the min_ack_delay transport parameter * wire: validate the min_ack_delay transport parameter --- fuzzing/transportparameters/fuzz.go | 8 ++++ internal/wire/transport_parameter_test.go | 56 +++++++++++++++++++---- internal/wire/transport_parameters.go | 31 ++++++++++--- 3 files changed, 78 insertions(+), 17 deletions(-) diff --git a/fuzzing/transportparameters/fuzz.go b/fuzzing/transportparameters/fuzz.go index 267979ce3..8bbe431af 100644 --- a/fuzzing/transportparameters/fuzz.go +++ b/fuzzing/transportparameters/fuzz.go @@ -93,5 +93,13 @@ func validateTransportParameters(tp *wire.TransportParameters, sentBy protocol.P if tp.PreferredAddress != nil && tp.PreferredAddress.ConnectionID.Len() > 20 { return fmt.Errorf("invalid preferred_address connection ID length: %s", tp.PreferredAddress.ConnectionID) } + if tp.MinAckDelay != nil { + if *tp.MinAckDelay < 0 { + return fmt.Errorf("negative min_ack_delay: %s", *tp.MinAckDelay) + } + if *tp.MinAckDelay > tp.MaxAckDelay { + return fmt.Errorf("min_ack_delay (%s) is greater than max_ack_delay (%s)", *tp.MinAckDelay, tp.MaxAckDelay) + } + } return nil } diff --git a/internal/wire/transport_parameter_test.go b/internal/wire/transport_parameter_test.go index 040937a43..c44561cbf 100644 --- a/internal/wire/transport_parameter_test.go +++ b/internal/wire/transport_parameter_test.go @@ -32,6 +32,7 @@ func appendInitialSourceConnectionID(b []byte) []byte { func TestTransportParametersStringRepresentation(t *testing.T) { rcid := protocol.ParseConnectionID([]byte{0xde, 0xad, 0xc0, 0xde}) + minAckDelay := 42 * time.Millisecond p := &TransportParameters{ InitialMaxStreamDataBidiLocal: 1234, InitialMaxStreamDataBidiRemote: 2345, @@ -49,8 +50,9 @@ func TestTransportParametersStringRepresentation(t *testing.T) { ActiveConnectionIDLimit: 123, MaxDatagramFrameSize: 876, EnableResetStreamAt: true, + MinAckDelay: &minAckDelay, } - expected := "&wire.TransportParameters{OriginalDestinationConnectionID: deadbeef, InitialSourceConnectionID: decafbad, RetrySourceConnectionID: deadc0de, InitialMaxStreamDataBidiLocal: 1234, InitialMaxStreamDataBidiRemote: 2345, InitialMaxStreamDataUni: 3456, InitialMaxData: 4567, MaxBidiStreamNum: 1337, MaxUniStreamNum: 7331, MaxIdleTimeout: 42s, AckDelayExponent: 14, MaxAckDelay: 37ms, ActiveConnectionIDLimit: 123, StatelessResetToken: 0x112233445566778899aabbccddeeff00, MaxDatagramFrameSize: 876, EnableResetStreamAt: true}" + expected := "&wire.TransportParameters{OriginalDestinationConnectionID: deadbeef, InitialSourceConnectionID: decafbad, RetrySourceConnectionID: deadc0de, InitialMaxStreamDataBidiLocal: 1234, InitialMaxStreamDataBidiRemote: 2345, InitialMaxStreamDataUni: 3456, InitialMaxData: 4567, MaxBidiStreamNum: 1337, MaxUniStreamNum: 7331, MaxIdleTimeout: 42s, AckDelayExponent: 14, MaxAckDelay: 37ms, ActiveConnectionIDLimit: 123, StatelessResetToken: 0x112233445566778899aabbccddeeff00, MaxDatagramFrameSize: 876, EnableResetStreamAt: true, MinAckDelay: 42ms}" require.Equal(t, expected, p.String()) } @@ -78,6 +80,7 @@ func TestMarshalAndUnmarshalTransportParameters(t *testing.T) { var token protocol.StatelessResetToken rand.Read(token[:]) rcid := protocol.ParseConnectionID([]byte{0xde, 0xad, 0xc0, 0xde}) + minAckDelay := 42 * time.Millisecond params := &TransportParameters{ InitialMaxStreamDataBidiLocal: protocol.ByteCount(getRandomValue()), InitialMaxStreamDataBidiRemote: protocol.ByteCount(getRandomValue()), @@ -97,6 +100,7 @@ func TestMarshalAndUnmarshalTransportParameters(t *testing.T) { MaxUDPPayloadSize: 1200 + protocol.ByteCount(getRandomValueUpTo(quicvarint.Max-1200)), MaxDatagramFrameSize: protocol.ByteCount(getRandomValue()), EnableResetStreamAt: getRandomValue()%2 == 0, + MinAckDelay: &minAckDelay, } data := params.Marshal(protocol.PerspectiveServer) @@ -120,11 +124,15 @@ func TestMarshalAndUnmarshalTransportParameters(t *testing.T) { require.Equal(t, params.MaxUDPPayloadSize, p.MaxUDPPayloadSize) require.Equal(t, params.MaxDatagramFrameSize, p.MaxDatagramFrameSize) require.Equal(t, params.EnableResetStreamAt, p.EnableResetStreamAt) + require.NotNil(t, p.MinAckDelay) + require.Equal(t, minAckDelay, *p.MinAckDelay) } func TestMarshalAdditionalTransportParameters(t *testing.T) { origAdditionalTransportParametersClient := AdditionalTransportParametersClient - t.Cleanup(func() { AdditionalTransportParametersClient = origAdditionalTransportParametersClient }) + t.Cleanup(func() { + AdditionalTransportParametersClient = origAdditionalTransportParametersClient + }) AdditionalTransportParametersClient = map[uint64][]byte{1337: []byte("foobar")} result := quicvarint.Append([]byte{}, 1337) @@ -136,24 +144,24 @@ func TestMarshalAdditionalTransportParameters(t *testing.T) { require.False(t, bytes.Contains(params.Marshal(protocol.PerspectiveServer), result)) } -func TestMarshalWithoutRetrySourceConnectionID(t *testing.T) { +func TestMarshalRetrySourceConnectionID(t *testing.T) { + // no retry source connection ID data := (&TransportParameters{ StatelessResetToken: &protocol.StatelessResetToken{}, ActiveConnectionIDLimit: 2, }).Marshal(protocol.PerspectiveServer) - p := &TransportParameters{} + var p TransportParameters require.NoError(t, p.Unmarshal(data, protocol.PerspectiveServer)) require.Nil(t, p.RetrySourceConnectionID) -} -func TestMarshalZeroLengthRetrySourceConnectionID(t *testing.T) { + // zero-length retry source connection ID rcid := protocol.ParseConnectionID([]byte{}) - data := (&TransportParameters{ + data = (&TransportParameters{ RetrySourceConnectionID: &rcid, StatelessResetToken: &protocol.StatelessResetToken{}, ActiveConnectionIDLimit: 2, }).Marshal(protocol.PerspectiveServer) - p := &TransportParameters{} + p = TransportParameters{} require.NoError(t, p.Unmarshal(data, protocol.PerspectiveServer)) require.NotNil(t, p.RetrySourceConnectionID) require.Zero(t, p.RetrySourceConnectionID.Len()) @@ -163,7 +171,7 @@ func TestTransportParameterNoMaxAckDelayIfDefault(t *testing.T) { const num = 1000 var defaultLen, dataLen int maxAckDelay := protocol.DefaultMaxAckDelay + time.Millisecond - for i := 0; i < num; i++ { + for range num { dataDefault := (&TransportParameters{ MaxAckDelay: protocol.DefaultMaxAckDelay, StatelessResetToken: &protocol.StatelessResetToken{}, @@ -184,7 +192,7 @@ func TestTransportParameterNoMaxAckDelayIfDefault(t *testing.T) { func TestTransportParameterNoAckDelayExponentIfDefault(t *testing.T) { const num = 1000 var defaultLen, dataLen int - for i := 0; i < num; i++ { + for range num { dataDefault := (&TransportParameters{ AckDelayExponent: protocol.DefaultAckDelayExponent, StatelessResetToken: &protocol.StatelessResetToken{}, @@ -383,6 +391,34 @@ func TestTransportParameterErrors(t *testing.T) { perspective: protocol.PerspectiveClient, expectedErrMsg: "wrong length for reset_stream_at: 1 (expected empty)", }, + { + name: "min ack delay is greater than max ack delay", + data: func() []byte { + b := quicvarint.Append(nil, uint64(minAckDelayParameterID)) + b = quicvarint.Append(b, uint64(quicvarint.Len(42001))) + b = quicvarint.Append(b, 42001) // 42001 microseconds + b = quicvarint.Append(b, uint64(maxAckDelayParameterID)) + b = quicvarint.Append(b, uint64(quicvarint.Len(42))) + b = quicvarint.Append(b, 42) // 42 microseconds + return appendInitialSourceConnectionID(b) + }(), + perspective: protocol.PerspectiveClient, + expectedErrMsg: "min_ack_delay (42.001ms) is greater than max_ack_delay (42ms)", + }, + { + name: "huge min ack delay value", + data: func() []byte { + b := quicvarint.Append(nil, uint64(minAckDelayParameterID)) + b = quicvarint.Append(b, uint64(quicvarint.Len(quicvarint.Max))) + b = quicvarint.Append(b, quicvarint.Max) + b = quicvarint.Append(b, uint64(maxAckDelayParameterID)) + b = quicvarint.Append(b, uint64(quicvarint.Len(42))) + b = quicvarint.Append(b, 42) // 42 microseconds + return appendInitialSourceConnectionID(b) + }(), + perspective: protocol.PerspectiveClient, + expectedErrMsg: "min_ack_delay (2562047h47m16.854775807s) is greater than max_ack_delay (42ms)", + }, } for _, tt := range tests { diff --git a/internal/wire/transport_parameters.go b/internal/wire/transport_parameters.go index 2437892a0..ff101b6e7 100644 --- a/internal/wire/transport_parameters.go +++ b/internal/wire/transport_parameters.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "math" "net/netip" "slices" "time" @@ -47,6 +48,8 @@ const ( maxDatagramFrameSizeParameterID transportParameterID = 0x20 // https://datatracker.ietf.org/doc/draft-ietf-quic-reliable-stream-reset/06/ resetStreamAtParameterID transportParameterID = 0x17f7586d2cb571 + // https://datatracker.ietf.org/doc/draft-ietf-quic-ack-frequency/11/ + minAckDelayParameterID transportParameterID = 0xff04de1b ) // PreferredAddress is the value encoding in the preferred_address transport parameter @@ -86,6 +89,7 @@ type TransportParameters struct { MaxDatagramFrameSize protocol.ByteCount // RFC 9221 EnableResetStreamAt bool // https://datatracker.ietf.org/doc/draft-ietf-quic-reliable-stream-reset/06/ + MinAckDelay *time.Duration } // Unmarshal the transport parameters @@ -106,12 +110,12 @@ func (p *TransportParameters) unmarshal(b []byte, sentBy protocol.Perspective, f var ( readOriginalDestinationConnectionID bool readInitialSourceConnectionID bool - readActiveConnectionIDLimit bool ) p.AckDelayExponent = protocol.DefaultAckDelayExponent p.MaxAckDelay = protocol.DefaultMaxAckDelay p.MaxDatagramFrameSize = protocol.InvalidByteCount + p.ActiveConnectionIDLimit = protocol.DefaultActiveConnectionIDLimit for len(b) > 0 { paramIDInt, l, err := quicvarint.Parse(b) @@ -130,9 +134,6 @@ func (p *TransportParameters) unmarshal(b []byte, sentBy protocol.Perspective, f } parameterIDs = append(parameterIDs, paramID) switch paramID { - case activeConnectionIDLimitParameterID: - readActiveConnectionIDLimit = true - fallthrough case maxIdleTimeoutParameterID, maxUDPPayloadSizeParameterID, initialMaxDataParameterID, @@ -143,7 +144,9 @@ func (p *TransportParameters) unmarshal(b []byte, sentBy protocol.Perspective, f initialMaxStreamsUniParameterID, maxAckDelayParameterID, maxDatagramFrameSizeParameterID, - ackDelayExponentParameterID: + ackDelayExponentParameterID, + activeConnectionIDLimitParameterID, + minAckDelayParameterID: if err := p.readNumericTransportParameter(b, paramID, int(paramLen)); err != nil { return err } @@ -212,8 +215,9 @@ func (p *TransportParameters) unmarshal(b []byte, sentBy protocol.Perspective, f } } - if !readActiveConnectionIDLimit { - p.ActiveConnectionIDLimit = protocol.DefaultActiveConnectionIDLimit + // min_ack_delay must be less or equal to max_ack_delay + if p.MinAckDelay != nil && *p.MinAckDelay > p.MaxAckDelay { + return fmt.Errorf("min_ack_delay (%s) is greater than max_ack_delay (%s)", *p.MinAckDelay, p.MaxAckDelay) } if !fromSessionTicket { if sentBy == protocol.PerspectiveServer && !readOriginalDestinationConnectionID { @@ -334,6 +338,12 @@ func (p *TransportParameters) readNumericTransportParameter(b []byte, paramID tr p.ActiveConnectionIDLimit = val case maxDatagramFrameSizeParameterID: p.MaxDatagramFrameSize = protocol.ByteCount(val) + case minAckDelayParameterID: + mad := time.Duration(val) * time.Microsecond + if mad < 0 { + mad = math.MaxInt64 + } + p.MinAckDelay = &mad default: return fmt.Errorf("TransportParameter BUG: transport parameter %d not found", paramID) } @@ -445,6 +455,9 @@ func (p *TransportParameters) Marshal(pers protocol.Perspective) []byte { b = quicvarint.Append(b, uint64(resetStreamAtParameterID)) b = quicvarint.Append(b, 0) } + if p.MinAckDelay != nil { + b = p.marshalVarintParam(b, minAckDelayParameterID, uint64(*p.MinAckDelay/time.Microsecond)) + } if pers == protocol.PerspectiveClient && len(AdditionalTransportParametersClient) > 0 { for k, v := range AdditionalTransportParametersClient { @@ -561,6 +574,10 @@ func (p *TransportParameters) String() string { } logString += ", EnableResetStreamAt: %t" logParams = append(logParams, p.EnableResetStreamAt) + if p.MinAckDelay != nil { + logString += ", MinAckDelay: %s" + logParams = append(logParams, *p.MinAckDelay) + } logString += "}" return fmt.Sprintf(logString, logParams...) }