From 9ee6efd50605fbe70b51b9336faab3d1db907e37 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sat, 8 Feb 2020 18:04:49 +0100 Subject: [PATCH] change the transport parameter format to varint --- .../handshake/transport_parameter_test.go | 107 +++++++----------- internal/handshake/transport_parameters.go | 63 ++++------- 2 files changed, 65 insertions(+), 105 deletions(-) diff --git a/internal/handshake/transport_parameter_test.go b/internal/handshake/transport_parameter_test.go index b72b1e3a..f40d92b8 100644 --- a/internal/handshake/transport_parameter_test.go +++ b/internal/handshake/transport_parameter_test.go @@ -2,7 +2,6 @@ package handshake import ( "bytes" - "encoding/binary" "fmt" "math" "math/rand" @@ -16,12 +15,6 @@ import ( ) var _ = Describe("Transport Parameters", func() { - prependLength := func(tp []byte) []byte { - data := make([]byte, 2) - binary.BigEndian.PutUint16(data, uint16(len(tp))) - return append(data, tp...) - } - getRandomValue := func() uint64 { maxVals := []int64{math.MaxUint8 / 4, math.MaxUint16 / 4, math.MaxUint32 / 4, math.MaxUint64 / 4} return uint64(rand.Int63n(maxVals[int(rand.Int31n(4))])) @@ -103,42 +96,31 @@ var _ = Describe("Transport Parameters", func() { Expect(p.ActiveConnectionIDLimit).To(Equal(params.ActiveConnectionIDLimit)) }) - It("errors if the transport parameters are too short to contain the length", func() { - Expect((&TransportParameters{}).Unmarshal([]byte{0}, protocol.PerspectiveClient)).To(MatchError("TRANSPORT_PARAMETER_ERROR: transport parameter data too short")) - }) - - It("errors if the transport parameters are too short to contain the length", func() { - data := make([]byte, 2) - binary.BigEndian.PutUint16(data, 42) - data = append(data, make([]byte, 41)...) - Expect((&TransportParameters{}).Unmarshal(data, protocol.PerspectiveClient)).To(MatchError("TRANSPORT_PARAMETER_ERROR: expected transport parameters to be 42 bytes long, have 41")) - }) - It("errors when the stateless_reset_token has the wrong length", func() { b := &bytes.Buffer{} - utils.BigEndian.WriteUint16(b, uint16(statelessResetTokenParameterID)) - utils.BigEndian.WriteUint16(b, 15) + utils.WriteVarInt(b, uint64(statelessResetTokenParameterID)) + utils.WriteVarInt(b, 15) b.Write(make([]byte, 15)) p := &TransportParameters{} - Expect(p.Unmarshal(prependLength(b.Bytes()), protocol.PerspectiveServer)).To(MatchError("TRANSPORT_PARAMETER_ERROR: wrong length for stateless_reset_token: 15 (expected 16)")) + Expect(p.Unmarshal(b.Bytes(), protocol.PerspectiveServer)).To(MatchError("TRANSPORT_PARAMETER_ERROR: wrong length for stateless_reset_token: 15 (expected 16)")) }) It("errors when the max_packet_size is too small", func() { b := &bytes.Buffer{} - utils.BigEndian.WriteUint16(b, uint16(maxPacketSizeParameterID)) - utils.BigEndian.WriteUint16(b, uint16(utils.VarIntLen(1199))) + utils.WriteVarInt(b, uint64(maxPacketSizeParameterID)) + utils.WriteVarInt(b, uint64(utils.VarIntLen(1199))) utils.WriteVarInt(b, 1199) p := &TransportParameters{} - Expect(p.Unmarshal(prependLength(b.Bytes()), protocol.PerspectiveServer)).To(MatchError("TRANSPORT_PARAMETER_ERROR: invalid value for max_packet_size: 1199 (minimum 1200)")) + Expect(p.Unmarshal(b.Bytes(), protocol.PerspectiveServer)).To(MatchError("TRANSPORT_PARAMETER_ERROR: invalid value for max_packet_size: 1199 (minimum 1200)")) }) It("errors when disable_active_migration has content", func() { b := &bytes.Buffer{} - utils.BigEndian.WriteUint16(b, uint16(disableActiveMigrationParameterID)) - utils.BigEndian.WriteUint16(b, 6) + utils.WriteVarInt(b, uint64(disableActiveMigrationParameterID)) + utils.WriteVarInt(b, 6) b.Write([]byte("foobar")) p := &TransportParameters{} - Expect(p.Unmarshal(prependLength(b.Bytes()), protocol.PerspectiveServer)).To(MatchError("TRANSPORT_PARAMETER_ERROR: wrong length for disable_active_migration: 6 (expected empty)")) + Expect(p.Unmarshal(b.Bytes(), protocol.PerspectiveServer)).To(MatchError("TRANSPORT_PARAMETER_ERROR: wrong length for disable_active_migration: 6 (expected empty)")) }) It("errors when the max_ack_delay is too large", func() { @@ -151,13 +133,15 @@ var _ = Describe("Transport Parameters", func() { const num = 1000 var defaultLen, dataLen int // marshal 1000 times to average out the greasing transport parameter + maxAckDelay := protocol.DefaultMaxAckDelay + time.Millisecond for i := 0; i < num; i++ { dataDefault := (&TransportParameters{MaxAckDelay: protocol.DefaultMaxAckDelay}).Marshal() defaultLen += len(dataDefault) - data := (&TransportParameters{MaxAckDelay: protocol.DefaultMaxAckDelay + time.Millisecond}).Marshal() + data := (&TransportParameters{MaxAckDelay: maxAckDelay}).Marshal() dataLen += len(data) } - Expect(float32(dataLen) / num).To(BeNumerically("~", float32(defaultLen)/num+2 /* parameter ID */ +2 /* length field */ +1 /* value */, 1)) + entryLen := utils.VarIntLen(uint64(ackDelayExponentParameterID)) /* parameter id */ + utils.VarIntLen(uint64(utils.VarIntLen(uint64(maxAckDelay.Milliseconds())))) /*length */ + utils.VarIntLen(uint64(maxAckDelay.Milliseconds())) /* value */ + Expect(float32(dataLen) / num).To(BeNumerically("~", float32(defaultLen)/num+float32(entryLen), 1)) }) It("errors when the ack_delay_exponenent is too large", func() { @@ -176,7 +160,8 @@ var _ = Describe("Transport Parameters", func() { data := (&TransportParameters{AckDelayExponent: protocol.DefaultAckDelayExponent + 1}).Marshal() dataLen += len(data) } - Expect(float32(dataLen) / num).To(BeNumerically("~", float32(defaultLen)/num+2 /* parameter ID */ +2 /* length field */ +1 /* value */, 1)) + entryLen := utils.VarIntLen(uint64(ackDelayExponentParameterID)) /* parameter id */ + utils.VarIntLen(uint64(utils.VarIntLen(protocol.DefaultAckDelayExponent+1))) /* length */ + utils.VarIntLen(protocol.DefaultAckDelayExponent+1) /* value */ + Expect(float32(dataLen) / num).To(BeNumerically("~", float32(defaultLen)/num+float32(entryLen), 1)) }) It("sets the default value for the ack_delay_exponent, when no value was sent", func() { @@ -188,13 +173,13 @@ var _ = Describe("Transport Parameters", func() { It("errors when the varint value has the wrong length", func() { b := &bytes.Buffer{} - utils.BigEndian.WriteUint16(b, uint16(initialMaxStreamDataBidiLocalParameterID)) - utils.BigEndian.WriteUint16(b, 2) + utils.WriteVarInt(b, uint64(initialMaxStreamDataBidiLocalParameterID)) + utils.WriteVarInt(b, 2) val := uint64(0xdeadbeef) Expect(utils.VarIntLen(val)).ToNot(BeEquivalentTo(2)) utils.WriteVarInt(b, val) p := &TransportParameters{} - err := p.Unmarshal(prependLength(b.Bytes()), protocol.PerspectiveServer) + err := p.Unmarshal(b.Bytes(), protocol.PerspectiveServer) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("TRANSPORT_PARAMETER_ERROR: inconsistent transport parameter length")) }) @@ -202,30 +187,30 @@ var _ = Describe("Transport Parameters", func() { It("handles max_ack_delays that decode to a negative duration", func() { b := &bytes.Buffer{} val := uint64(math.MaxUint64) / 5 - utils.BigEndian.WriteUint16(b, uint16(maxAckDelayParameterID)) - utils.BigEndian.WriteUint16(b, uint16(utils.VarIntLen(val))) + utils.WriteVarInt(b, uint64(maxAckDelayParameterID)) + utils.WriteVarInt(b, uint64(utils.VarIntLen(val))) utils.WriteVarInt(b, val) p := &TransportParameters{} - Expect(p.Unmarshal(prependLength(b.Bytes()), protocol.PerspectiveServer)).To(Succeed()) + Expect(p.Unmarshal(b.Bytes(), protocol.PerspectiveServer)).To(Succeed()) Expect(p.MaxAckDelay).To(BeNumerically(">", 290*365*24*time.Hour)) }) It("skips unknown parameters", func() { b := &bytes.Buffer{} // write a known parameter - utils.BigEndian.WriteUint16(b, uint16(initialMaxStreamDataBidiLocalParameterID)) - utils.BigEndian.WriteUint16(b, uint16(utils.VarIntLen(0x1337))) + utils.WriteVarInt(b, uint64(initialMaxStreamDataBidiLocalParameterID)) + utils.WriteVarInt(b, uint64(utils.VarIntLen(0x1337))) utils.WriteVarInt(b, 0x1337) // write an unknown parameter - utils.BigEndian.WriteUint16(b, 0x42) - utils.BigEndian.WriteUint16(b, 6) + utils.WriteVarInt(b, 0x42) + utils.WriteVarInt(b, 6) b.Write([]byte("foobar")) // write a known parameter - utils.BigEndian.WriteUint16(b, uint16(initialMaxStreamDataBidiRemoteParameterID)) - utils.BigEndian.WriteUint16(b, uint16(utils.VarIntLen(0x42))) + utils.WriteVarInt(b, uint64(initialMaxStreamDataBidiRemoteParameterID)) + utils.WriteVarInt(b, uint64(utils.VarIntLen(0x42))) utils.WriteVarInt(b, 0x42) p := &TransportParameters{} - Expect(p.Unmarshal(prependLength(b.Bytes()), protocol.PerspectiveServer)).To(Succeed()) + Expect(p.Unmarshal(b.Bytes(), protocol.PerspectiveServer)).To(Succeed()) Expect(p.InitialMaxStreamDataBidiLocal).To(Equal(protocol.ByteCount(0x1337))) Expect(p.InitialMaxStreamDataBidiRemote).To(Equal(protocol.ByteCount(0x42))) }) @@ -233,40 +218,30 @@ var _ = Describe("Transport Parameters", func() { It("rejects duplicate parameters", func() { b := &bytes.Buffer{} // write first parameter - utils.BigEndian.WriteUint16(b, uint16(initialMaxStreamDataBidiLocalParameterID)) - utils.BigEndian.WriteUint16(b, uint16(utils.VarIntLen(0x1337))) + utils.WriteVarInt(b, uint64(initialMaxStreamDataBidiLocalParameterID)) + utils.WriteVarInt(b, uint64(utils.VarIntLen(0x1337))) utils.WriteVarInt(b, 0x1337) // write a second parameter - utils.BigEndian.WriteUint16(b, uint16(initialMaxStreamDataBidiRemoteParameterID)) - utils.BigEndian.WriteUint16(b, uint16(utils.VarIntLen(0x42))) + utils.WriteVarInt(b, uint64(initialMaxStreamDataBidiRemoteParameterID)) + utils.WriteVarInt(b, uint64(utils.VarIntLen(0x42))) utils.WriteVarInt(b, 0x42) // write first parameter again - utils.BigEndian.WriteUint16(b, uint16(initialMaxStreamDataBidiLocalParameterID)) - utils.BigEndian.WriteUint16(b, uint16(utils.VarIntLen(0x1337))) + utils.WriteVarInt(b, uint64(initialMaxStreamDataBidiLocalParameterID)) + utils.WriteVarInt(b, uint64(utils.VarIntLen(0x1337))) utils.WriteVarInt(b, 0x1337) p := &TransportParameters{} - err := p.Unmarshal(prependLength(b.Bytes()), protocol.PerspectiveServer) + err := p.Unmarshal(b.Bytes(), protocol.PerspectiveServer) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("received duplicate transport parameter")) }) It("errors if there's not enough data to read", func() { b := &bytes.Buffer{} - utils.BigEndian.WriteUint16(b, 0x42) - utils.BigEndian.WriteUint16(b, 7) + utils.WriteVarInt(b, 0x42) + utils.WriteVarInt(b, 7) b.Write([]byte("foobar")) p := &TransportParameters{} - Expect(p.Unmarshal(prependLength(b.Bytes()), protocol.PerspectiveServer)).To(MatchError("TRANSPORT_PARAMETER_ERROR: remaining length (6) smaller than parameter length (7)")) - }) - - It("errors if there's unprocessed data after reading", func() { - b := &bytes.Buffer{} - utils.BigEndian.WriteUint16(b, uint16(initialMaxStreamDataBidiLocalParameterID)) - utils.BigEndian.WriteUint16(b, uint16(utils.VarIntLen(0x1337))) - utils.WriteVarInt(b, 0x1337) - b.Write([]byte("foo")) - p := &TransportParameters{} - Expect(p.Unmarshal(prependLength(b.Bytes()), protocol.PerspectiveServer)).To(MatchError("TRANSPORT_PARAMETER_ERROR: should have read all data. Still have 3 bytes")) + Expect(p.Unmarshal(b.Bytes(), protocol.PerspectiveServer)).To(MatchError("TRANSPORT_PARAMETER_ERROR: remaining length (6) smaller than parameter length (7)")) }) It("errors if the client sent a stateless_reset_token", func() { @@ -324,10 +299,10 @@ var _ = Describe("Transport Parameters", func() { } for i := 1; i < len(raw); i++ { buf := &bytes.Buffer{} - utils.BigEndian.WriteUint16(buf, uint16(preferredAddressParamaterID)) - buf.Write(prependLength(raw[:i])) + utils.WriteVarInt(buf, uint64(preferredAddressParamaterID)) + buf.Write(raw[:i]) p := &TransportParameters{} - Expect(p.Unmarshal(prependLength(buf.Bytes()), protocol.PerspectiveServer)).ToNot(Succeed()) + Expect(p.Unmarshal(buf.Bytes(), protocol.PerspectiveServer)).ToNot(Succeed()) } }) }) diff --git a/internal/handshake/transport_parameters.go b/internal/handshake/transport_parameters.go index 3348fac5..7665a757 100644 --- a/internal/handshake/transport_parameters.go +++ b/internal/handshake/transport_parameters.go @@ -2,7 +2,6 @@ package handshake import ( "bytes" - "encoding/binary" "errors" "fmt" "io" @@ -23,7 +22,7 @@ func init() { rand.Seed(time.Now().UTC().UnixNano()) } -type transportParameterID uint16 +type transportParameterID uint64 const ( originalConnectionIDParameterID transportParameterID = 0x0 @@ -88,25 +87,23 @@ func (p *TransportParameters) Unmarshal(data []byte, sentBy protocol.Perspective } func (p *TransportParameters) unmarshal(data []byte, sentBy protocol.Perspective) error { - if len(data) < 2 { - return errors.New("transport parameter data too short") - } - length := binary.BigEndian.Uint16(data[:2]) - if len(data)-2 < int(length) { - return fmt.Errorf("expected transport parameters to be %d bytes long, have %d", length, len(data)-2) - } - // needed to check that every parameter is only sent at most once var parameterIDs []transportParameterID var readAckDelayExponent bool var readMaxAckDelay bool - r := bytes.NewReader(data[2:]) - for r.Len() >= 4 { - paramIDInt, _ := utils.BigEndian.ReadUint16(r) + r := bytes.NewReader(data) + for r.Len() > 0 { + paramIDInt, err := utils.ReadVarInt(r) + if err != nil { + return err + } paramID := transportParameterID(paramIDInt) - paramLen, _ := utils.BigEndian.ReadUint16(r) + paramLen, err := utils.ReadVarInt(r) + if err != nil { + return err + } parameterIDs = append(parameterIDs, paramID) switch paramID { case ackDelayExponentParameterID: @@ -187,9 +184,6 @@ func (p *TransportParameters) unmarshal(data []byte, sentBy protocol.Perspective } } - if r.Len() != 0 { - return fmt.Errorf("should have read all data. Still have %d bytes", r.Len()) - } return nil } @@ -293,14 +287,13 @@ func (p *TransportParameters) readNumericTransportParameter( // Marshal the transport parameters func (p *TransportParameters) Marshal() []byte { b := &bytes.Buffer{} - b.Write([]byte{0, 0}) // length. Will be replaced later //add a greased value - utils.BigEndian.WriteUint16(b, uint16(27+31*rand.Intn(100))) + utils.WriteVarInt(b, uint64(27+31*rand.Intn(100))) length := rand.Intn(16) randomData := make([]byte, length) rand.Read(randomData) - utils.BigEndian.WriteUint16(b, uint16(length)) + utils.WriteVarInt(b, uint64(length)) b.Write(randomData) // initial_max_stream_data_bidi_local @@ -331,17 +324,17 @@ func (p *TransportParameters) Marshal() []byte { } // disable_active_migration if p.DisableActiveMigration { - utils.BigEndian.WriteUint16(b, uint16(disableActiveMigrationParameterID)) - utils.BigEndian.WriteUint16(b, 0) + utils.WriteVarInt(b, uint64(disableActiveMigrationParameterID)) + utils.WriteVarInt(b, 0) } if p.StatelessResetToken != nil { - utils.BigEndian.WriteUint16(b, uint16(statelessResetTokenParameterID)) - utils.BigEndian.WriteUint16(b, 16) + utils.WriteVarInt(b, uint64(statelessResetTokenParameterID)) + utils.WriteVarInt(b, 16) b.Write(p.StatelessResetToken[:]) } if p.PreferredAddress != nil { - utils.BigEndian.WriteUint16(b, uint16(preferredAddressParamaterID)) - utils.BigEndian.WriteUint16(b, 4+2+16+2+1+uint16(p.PreferredAddress.ConnectionID.Len())+16) + utils.WriteVarInt(b, uint64(preferredAddressParamaterID)) + utils.WriteVarInt(b, 4+2+16+2+1+uint64(p.PreferredAddress.ConnectionID.Len())+16) ipv4 := p.PreferredAddress.IPv4 b.Write(ipv4[len(ipv4)-4:]) utils.BigEndian.WriteUint16(b, p.PreferredAddress.IPv4Port) @@ -352,22 +345,19 @@ func (p *TransportParameters) Marshal() []byte { b.Write(p.PreferredAddress.StatelessResetToken[:]) } if p.OriginalConnectionID.Len() > 0 { - utils.BigEndian.WriteUint16(b, uint16(originalConnectionIDParameterID)) - utils.BigEndian.WriteUint16(b, uint16(p.OriginalConnectionID.Len())) + utils.WriteVarInt(b, uint64(originalConnectionIDParameterID)) + utils.WriteVarInt(b, uint64(p.OriginalConnectionID.Len())) b.Write(p.OriginalConnectionID.Bytes()) } // active_connection_id_limit p.marshalVarintParam(b, activeConnectionIDLimitParameterID, p.ActiveConnectionIDLimit) - - data := b.Bytes() - binary.BigEndian.PutUint16(data[:2], uint16(b.Len()-2)) - return data + return b.Bytes() } func (p *TransportParameters) marshalVarintParam(b *bytes.Buffer, id transportParameterID, val uint64) { - utils.BigEndian.WriteUint16(b, uint16(id)) - utils.BigEndian.WriteUint16(b, uint16(utils.VarIntLen(val))) + utils.WriteVarInt(b, uint64(id)) + utils.WriteVarInt(b, uint64(utils.VarIntLen(val))) utils.WriteVarInt(b, val) } @@ -381,8 +371,6 @@ func (p *TransportParameters) marshalVarintParam(b *bytes.Buffer, id transportPa // For convenience, we use the same format that we also use for sending the transport parameters. func (p *TransportParameters) MarshalForSessionTicket(b *bytes.Buffer) { utils.WriteVarInt(b, transportParameterMarshalingVersion) - startLen := b.Len() - b.Write([]byte{0, 0}) // length. Will be replaced later // initial_max_stream_data_bidi_local p.marshalVarintParam(b, initialMaxStreamDataBidiLocalParameterID, uint64(p.InitialMaxStreamDataBidiLocal)) @@ -396,9 +384,6 @@ func (p *TransportParameters) MarshalForSessionTicket(b *bytes.Buffer) { p.marshalVarintParam(b, initialMaxStreamsBidiParameterID, uint64(p.MaxBidiStreamNum)) // initial_max_uni_streams p.marshalVarintParam(b, initialMaxStreamsUniParameterID, uint64(p.MaxUniStreamNum)) - - // replace the length - binary.BigEndian.PutUint16(b.Bytes()[startLen:startLen+2], uint16(b.Len()-2-startLen)) } // UnmarshalFromSessionTicket unmarshals transport parameters from a session ticket.