From f847c5422d3c5c3c235f75f5f295a263a0a20ad1 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Wed, 8 May 2019 12:43:29 +0900 Subject: [PATCH] implement parsing and writing of the max_ack_delay transport parameter --- .../handshake/transport_parameter_test.go | 21 ++++++++++-- internal/handshake/transport_parameters.go | 32 +++++++++++++++++-- internal/protocol/protocol.go | 7 ++++ 3 files changed, 55 insertions(+), 5 deletions(-) diff --git a/internal/handshake/transport_parameter_test.go b/internal/handshake/transport_parameter_test.go index 84cc932e..930e1b6d 100644 --- a/internal/handshake/transport_parameter_test.go +++ b/internal/handshake/transport_parameter_test.go @@ -31,9 +31,10 @@ var _ = Describe("Transport Parameters", func() { IdleTimeout: 42 * time.Second, OriginalConnectionID: protocol.ConnectionID{0xde, 0xad, 0xbe, 0xef}, AckDelayExponent: 14, + MaxAckDelay: 37 * time.Millisecond, StatelessResetToken: &[16]byte{0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff, 0x00}, } - Expect(p.String()).To(Equal("&handshake.TransportParameters{OriginalConnectionID: 0xdeadbeef, InitialMaxStreamDataBidiLocal: 0x1234, InitialMaxStreamDataBidiRemote: 0x2345, InitialMaxStreamDataUni: 0x3456, InitialMaxData: 0x4567, MaxBidiStreams: 1337, MaxUniStreams: 7331, IdleTimeout: 42s, AckDelayExponent: 14, StatelessResetToken: 0x112233445566778899aabbccddeeff00}")) + Expect(p.String()).To(Equal("&handshake.TransportParameters{OriginalConnectionID: 0xdeadbeef, InitialMaxStreamDataBidiLocal: 0x1234, InitialMaxStreamDataBidiRemote: 0x2345, InitialMaxStreamDataUni: 0x3456, InitialMaxData: 0x4567, MaxBidiStreams: 1337, MaxUniStreams: 7331, IdleTimeout: 42s, AckDelayExponent: 14, MaxAckDelay: 37ms, StatelessResetToken: 0x112233445566778899aabbccddeeff00}")) }) It("has a string representation, if there's no stateless reset token", func() { @@ -47,8 +48,9 @@ var _ = Describe("Transport Parameters", func() { IdleTimeout: 42 * time.Second, OriginalConnectionID: protocol.ConnectionID{0xde, 0xad, 0xbe, 0xef}, AckDelayExponent: 14, + MaxAckDelay: 37 * time.Second, } - Expect(p.String()).To(Equal("&handshake.TransportParameters{OriginalConnectionID: 0xdeadbeef, InitialMaxStreamDataBidiLocal: 0x1234, InitialMaxStreamDataBidiRemote: 0x2345, InitialMaxStreamDataUni: 0x3456, InitialMaxData: 0x4567, MaxBidiStreams: 1337, MaxUniStreams: 7331, IdleTimeout: 42s, AckDelayExponent: 14}")) + Expect(p.String()).To(Equal("&handshake.TransportParameters{OriginalConnectionID: 0xdeadbeef, InitialMaxStreamDataBidiLocal: 0x1234, InitialMaxStreamDataBidiRemote: 0x2345, InitialMaxStreamDataUni: 0x3456, InitialMaxData: 0x4567, MaxBidiStreams: 1337, MaxUniStreams: 7331, IdleTimeout: 42s, AckDelayExponent: 14, MaxAckDelay: 37s}")) }) getRandomValue := func() uint64 { @@ -72,6 +74,7 @@ var _ = Describe("Transport Parameters", func() { StatelessResetToken: &token, OriginalConnectionID: protocol.ConnectionID{0xde, 0xad, 0xbe, 0xef}, AckDelayExponent: 13, + MaxAckDelay: 42 * time.Millisecond, } data := params.Marshal() @@ -88,6 +91,7 @@ var _ = Describe("Transport Parameters", func() { Expect(p.StatelessResetToken).To(Equal(params.StatelessResetToken)) Expect(p.OriginalConnectionID).To(Equal(protocol.ConnectionID{0xde, 0xad, 0xbe, 0xef})) Expect(p.AckDelayExponent).To(Equal(uint8(13))) + Expect(p.MaxAckDelay).To(Equal(42 * time.Millisecond)) }) It("errors if the transport parameters are too short to contain the length", func() { @@ -128,6 +132,19 @@ var _ = Describe("Transport Parameters", func() { Expect(p.Unmarshal(prependLength(b.Bytes()), protocol.PerspectiveServer)).To(MatchError("wrong length for disable_migration: 6 (expected empty)")) }) + It("errors when the max_ack_delay is too large", func() { + data := (&TransportParameters{MaxAckDelay: 1 << 14 * time.Millisecond}).Marshal() + p := &TransportParameters{} + Expect(p.Unmarshal(data, protocol.PerspectiveServer)).To(MatchError("invalid value for max_ack_delay: 16384ms (maximum 16383ms)")) + }) + + It("doesn't send the max_ack_delay, if it has the default value", func() { + dataDefault := (&TransportParameters{MaxAckDelay: protocol.DefaultMaxAckDelay}).Marshal() + defaultLen := len(dataDefault) + data := (&TransportParameters{MaxAckDelay: protocol.DefaultMaxAckDelay + time.Millisecond}).Marshal() + Expect(len(data)).To(Equal(defaultLen + 2 /* parameter ID */ + 2 /* length field */ + 1 /* value */)) + }) + It("errors when the ack_delay_exponenent is too large", func() { data := (&TransportParameters{AckDelayExponent: 21}).Marshal() p := &TransportParameters{} diff --git a/internal/handshake/transport_parameters.go b/internal/handshake/transport_parameters.go index 6810cbc3..70f748d2 100644 --- a/internal/handshake/transport_parameters.go +++ b/internal/handshake/transport_parameters.go @@ -27,6 +27,7 @@ const ( initialMaxStreamsBidiParameterID transportParameterID = 0x8 initialMaxStreamsUniParameterID transportParameterID = 0x9 ackDelayExponentParameterID transportParameterID = 0xa + maxAckDelayParameterID transportParameterID = 0xb disableMigrationParameterID transportParameterID = 0xc ) @@ -37,6 +38,7 @@ type TransportParameters struct { InitialMaxStreamDataUni protocol.ByteCount InitialMaxData protocol.ByteCount + MaxAckDelay time.Duration AckDelayExponent uint8 MaxPacketSize protocol.ByteCount @@ -65,6 +67,7 @@ func (p *TransportParameters) Unmarshal(data []byte, sentBy protocol.Perspective var parameterIDs []transportParameterID var readAckDelayExponent bool + var readMaxAckDelay bool r := bytes.NewReader(data[2:]) for r.Len() >= 4 { @@ -75,7 +78,14 @@ func (p *TransportParameters) Unmarshal(data []byte, sentBy protocol.Perspective switch paramID { case ackDelayExponentParameterID: readAckDelayExponent = true - fallthrough + if err := p.readNumericTransportParameter(r, paramID, int(paramLen)); err != nil { + return err + } + case maxAckDelayParameterID: + readMaxAckDelay = true + if err := p.readNumericTransportParameter(r, paramID, int(paramLen)); err != nil { + return err + } case initialMaxStreamDataBidiLocalParameterID, initialMaxStreamDataBidiRemoteParameterID, initialMaxStreamDataUniParameterID, @@ -121,6 +131,9 @@ func (p *TransportParameters) Unmarshal(data []byte, sentBy protocol.Perspective if !readAckDelayExponent { p.AckDelayExponent = protocol.DefaultAckDelayExponent } + if !readMaxAckDelay { + p.MaxAckDelay = protocol.DefaultMaxAckDelay + } // check that every transport parameter was sent at most once sort.Slice(parameterIDs, func(i, j int) bool { return parameterIDs[i] < parameterIDs[j] }) @@ -174,6 +187,12 @@ func (p *TransportParameters) readNumericTransportParameter( return fmt.Errorf("invalid value for ack_delay_exponent: %d (maximum %d)", val, protocol.MaxAckDelayExponent) } p.AckDelayExponent = uint8(val) + case maxAckDelayParameterID: + maxAckDelay := time.Duration(val) * time.Millisecond + if maxAckDelay >= protocol.MaxMaxAckDelay { + return fmt.Errorf("invalid value for max_ack_delay: %dms (maximum %dms)", maxAckDelay/time.Millisecond, (protocol.MaxMaxAckDelay-time.Millisecond)/time.Millisecond) + } + p.MaxAckDelay = maxAckDelay default: return fmt.Errorf("TransportParameter BUG: transport parameter %d not found", paramID) } @@ -218,6 +237,13 @@ func (p *TransportParameters) Marshal() []byte { utils.BigEndian.WriteUint16(b, uint16(maxPacketSizeParameterID)) utils.BigEndian.WriteUint16(b, uint16(utils.VarIntLen(uint64(protocol.MaxReceivePacketSize)))) utils.WriteVarInt(b, uint64(protocol.MaxReceivePacketSize)) + // max_ack_delay + // Only send it if is different from the default value. + if p.MaxAckDelay != protocol.DefaultMaxAckDelay { + utils.BigEndian.WriteUint16(b, uint16(maxAckDelayParameterID)) + utils.BigEndian.WriteUint16(b, uint16(utils.VarIntLen(uint64(p.MaxAckDelay/time.Millisecond)))) + utils.WriteVarInt(b, uint64(p.MaxAckDelay/time.Millisecond)) + } // ack_delay_exponent // Only send it if is different from the default value. if p.AckDelayExponent != protocol.DefaultAckDelayExponent { @@ -249,8 +275,8 @@ func (p *TransportParameters) Marshal() []byte { // String returns a string representation, intended for logging. func (p *TransportParameters) String() string { - logString := "&handshake.TransportParameters{OriginalConnectionID: %s, InitialMaxStreamDataBidiLocal: %#x, InitialMaxStreamDataBidiRemote: %#x, InitialMaxStreamDataUni: %#x, InitialMaxData: %#x, MaxBidiStreams: %d, MaxUniStreams: %d, IdleTimeout: %s, AckDelayExponent: %d" - logParams := []interface{}{p.OriginalConnectionID, p.InitialMaxStreamDataBidiLocal, p.InitialMaxStreamDataBidiRemote, p.InitialMaxStreamDataUni, p.InitialMaxData, p.MaxBidiStreams, p.MaxUniStreams, p.IdleTimeout, p.AckDelayExponent} + logString := "&handshake.TransportParameters{OriginalConnectionID: %s, InitialMaxStreamDataBidiLocal: %#x, InitialMaxStreamDataBidiRemote: %#x, InitialMaxStreamDataUni: %#x, InitialMaxData: %#x, MaxBidiStreams: %d, MaxUniStreams: %d, IdleTimeout: %s, AckDelayExponent: %d, MaxAckDelay: %s" + logParams := []interface{}{p.OriginalConnectionID, p.InitialMaxStreamDataBidiLocal, p.InitialMaxStreamDataBidiRemote, p.InitialMaxStreamDataUni, p.InitialMaxData, p.MaxBidiStreams, p.MaxUniStreams, p.IdleTimeout, p.AckDelayExponent, p.MaxAckDelay} if p.StatelessResetToken != nil { // the client never sends a stateless reset token logString += ", StatelessResetToken: %#x" logParams = append(logParams, *p.StatelessResetToken) diff --git a/internal/protocol/protocol.go b/internal/protocol/protocol.go index 6e59afcb..c3e48fd9 100644 --- a/internal/protocol/protocol.go +++ b/internal/protocol/protocol.go @@ -2,6 +2,7 @@ package protocol import ( "fmt" + "time" ) // A PacketNumber in QUIC @@ -73,3 +74,9 @@ const DefaultAckDelayExponent = 3 // MaxAckDelayExponent is the maximum ack delay exponent const MaxAckDelayExponent = 20 + +// DefaultMaxAckDelay is the default max_ack_delay +const DefaultMaxAckDelay = 25 * time.Millisecond + +// MaxMaxAckDelay is the maximum max_ack_delay +const MaxMaxAckDelay = 1 << 14 * time.Millisecond