From a72efca38da3cfb07ec4b3dfc8d67418150566ba Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Tue, 5 Dec 2017 08:56:49 +0700 Subject: [PATCH 1/2] don't require the initial_max_stream_id in the transport parameters The draft was recently changed to make this value optional. --- internal/handshake/transport_parameter_test.go | 6 ------ internal/handshake/transport_parameters.go | 4 +--- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/internal/handshake/transport_parameter_test.go b/internal/handshake/transport_parameter_test.go index 676ba6be4..d97b55424 100644 --- a/internal/handshake/transport_parameter_test.go +++ b/internal/handshake/transport_parameter_test.go @@ -149,12 +149,6 @@ var _ = Describe("Transport Parameters", func() { Expect(err).To(MatchError("missing parameter")) }) - It("rejects the parameters if the initial_max_stream_id is missing", func() { - delete(parameters, initialMaxStreamIDParameterID) - _, err := readTransportParamters(paramsMapToList(parameters)) - Expect(err).To(MatchError("missing parameter")) - }) - It("rejects the parameters if the idle_timeout is missing", func() { delete(parameters, idleTimeoutParameterID) _, err := readTransportParamters(paramsMapToList(parameters)) diff --git a/internal/handshake/transport_parameters.go b/internal/handshake/transport_parameters.go index bda12c277..918171bc1 100644 --- a/internal/handshake/transport_parameters.go +++ b/internal/handshake/transport_parameters.go @@ -97,7 +97,6 @@ func readTransportParamters(paramsList []transportParameter) (*TransportParamete var foundInitialMaxStreamData bool var foundInitialMaxData bool - var foundInitialMaxStreamID bool var foundIdleTimeout bool for _, p := range paramsList { @@ -115,7 +114,6 @@ func readTransportParamters(paramsList []transportParameter) (*TransportParamete } params.ConnectionFlowControlWindow = protocol.ByteCount(binary.BigEndian.Uint32(p.Value)) case initialMaxStreamIDParameterID: - foundInitialMaxStreamID = true if len(p.Value) != 4 { return nil, fmt.Errorf("wrong length for initial_max_stream_id: %d (expected 4)", len(p.Value)) } @@ -134,7 +132,7 @@ func readTransportParamters(paramsList []transportParameter) (*TransportParamete } } - if !(foundInitialMaxStreamData && foundInitialMaxData && foundInitialMaxStreamID && foundIdleTimeout) { + if !(foundInitialMaxStreamData && foundInitialMaxData && foundIdleTimeout) { return nil, errors.New("missing parameter") } return params, nil From c30064bb51bc94e506381df5eec176cc14a806f1 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Tue, 5 Dec 2017 09:17:43 +0700 Subject: [PATCH 2/2] parse TLS extensions containing the initial_max_stream_id_uni parameter We're not sending the initial_max_stream_id_uni parameter, which implicitely sets this value to 0, i.e. the peer is not allowed to open unidirectional streams. --- internal/handshake/tls_extension.go | 15 ++++++------ .../tls_extension_handler_client_test.go | 10 ++++---- .../tls_extension_handler_server_test.go | 8 +++---- .../handshake/transport_parameter_test.go | 24 ++++++++++++------- internal/handshake/transport_parameters.go | 16 +++++++++---- 5 files changed, 44 insertions(+), 29 deletions(-) diff --git a/internal/handshake/tls_extension.go b/internal/handshake/tls_extension.go index 7e56e92bd..29d528368 100644 --- a/internal/handshake/tls_extension.go +++ b/internal/handshake/tls_extension.go @@ -9,13 +9,14 @@ type transportParameterID uint16 const quicTLSExtensionType = 26 const ( - initialMaxStreamDataParameterID transportParameterID = iota - initialMaxDataParameterID - initialMaxStreamIDParameterID - idleTimeoutParameterID - omitConnectionIDParameterID - maxPacketSizeParameterID - statelessResetTokenParameterID + initialMaxStreamDataParameterID transportParameterID = 0x0 + initialMaxDataParameterID transportParameterID = 0x1 + initialMaxStreamIDBiDiParameterID transportParameterID = 0x2 + idleTimeoutParameterID transportParameterID = 0x3 + omitConnectionIDParameterID transportParameterID = 0x4 + maxPacketSizeParameterID transportParameterID = 0x5 + statelessResetTokenParameterID transportParameterID = 0x6 + initialMaxStreamIDUniParameterID transportParameterID = 0x8 ) type transportParameter struct { diff --git a/internal/handshake/tls_extension_handler_client_test.go b/internal/handshake/tls_extension_handler_client_test.go index a19ee94c9..246477edb 100644 --- a/internal/handshake/tls_extension_handler_client_test.go +++ b/internal/handshake/tls_extension_handler_client_test.go @@ -70,11 +70,11 @@ var _ = Describe("TLS Extension Handler, for the client", func() { BeforeEach(func() { fakeBody = &tlsExtensionBody{data: []byte("foobar foobar")} parameters = map[transportParameterID][]byte{ - initialMaxStreamDataParameterID: []byte{0x11, 0x22, 0x33, 0x44}, - initialMaxDataParameterID: []byte{0x22, 0x33, 0x44, 0x55}, - initialMaxStreamIDParameterID: []byte{0x33, 0x44, 0x55, 0x66}, - idleTimeoutParameterID: []byte{0x13, 0x37}, - statelessResetTokenParameterID: bytes.Repeat([]byte{0}, 16), + initialMaxStreamDataParameterID: []byte{0x11, 0x22, 0x33, 0x44}, + initialMaxDataParameterID: []byte{0x22, 0x33, 0x44, 0x55}, + initialMaxStreamIDBiDiParameterID: []byte{0x33, 0x44, 0x55, 0x66}, + idleTimeoutParameterID: []byte{0x13, 0x37}, + statelessResetTokenParameterID: bytes.Repeat([]byte{0}, 16), } }) diff --git a/internal/handshake/tls_extension_handler_server_test.go b/internal/handshake/tls_extension_handler_server_test.go index 276895362..f497b23ec 100644 --- a/internal/handshake/tls_extension_handler_server_test.go +++ b/internal/handshake/tls_extension_handler_server_test.go @@ -72,10 +72,10 @@ var _ = Describe("TLS Extension Handler, for the server", func() { BeforeEach(func() { fakeBody = &tlsExtensionBody{data: []byte("foobar foobar")} parameters = map[transportParameterID][]byte{ - initialMaxStreamDataParameterID: []byte{0x11, 0x22, 0x33, 0x44}, - initialMaxDataParameterID: []byte{0x22, 0x33, 0x44, 0x55}, - initialMaxStreamIDParameterID: []byte{0x33, 0x44, 0x55, 0x66}, - idleTimeoutParameterID: []byte{0x13, 0x37}, + initialMaxStreamDataParameterID: []byte{0x11, 0x22, 0x33, 0x44}, + initialMaxDataParameterID: []byte{0x22, 0x33, 0x44, 0x55}, + initialMaxStreamIDBiDiParameterID: []byte{0x33, 0x44, 0x55, 0x66}, + idleTimeoutParameterID: []byte{0x13, 0x37}, } }) diff --git a/internal/handshake/transport_parameter_test.go b/internal/handshake/transport_parameter_test.go index d97b55424..17f6c26c6 100644 --- a/internal/handshake/transport_parameter_test.go +++ b/internal/handshake/transport_parameter_test.go @@ -115,10 +115,11 @@ var _ = Describe("Transport Parameters", func() { BeforeEach(func() { parameters = map[transportParameterID][]byte{ - initialMaxStreamDataParameterID: []byte{0x11, 0x22, 0x33, 0x44}, - initialMaxDataParameterID: []byte{0x22, 0x33, 0x44, 0x55}, - initialMaxStreamIDParameterID: []byte{0x33, 0x44, 0x55, 0x66}, - idleTimeoutParameterID: []byte{0x13, 0x37}, + initialMaxStreamDataParameterID: []byte{0x11, 0x22, 0x33, 0x44}, + initialMaxDataParameterID: []byte{0x22, 0x33, 0x44, 0x55}, + initialMaxStreamIDBiDiParameterID: []byte{0x33, 0x44, 0x55, 0x66}, + initialMaxStreamIDUniParameterID: []byte{0x44, 0x55, 0x66, 0x77}, + idleTimeoutParameterID: []byte{0x13, 0x37}, } }) It("reads parameters", func() { @@ -176,10 +177,16 @@ var _ = Describe("Transport Parameters", func() { Expect(err).To(MatchError("wrong length for initial_max_data: 3 (expected 4)")) }) - It("rejects the parameters if the initial_max_stream_id has the wrong length", func() { - parameters[initialMaxStreamIDParameterID] = []byte{0x11, 0x22, 0x33, 0x44, 0x55} // should be 4 bytes + It("rejects the parameters if the initial_max_stream_id_bidi has the wrong length", func() { + parameters[initialMaxStreamIDBiDiParameterID] = []byte{0x11, 0x22, 0x33, 0x44, 0x55} // should be 4 bytes _, err := readTransportParamters(paramsMapToList(parameters)) - Expect(err).To(MatchError("wrong length for initial_max_stream_id: 5 (expected 4)")) + Expect(err).To(MatchError("wrong length for initial_max_stream_id_bidi: 5 (expected 4)")) + }) + + It("rejects the parameters if the initial_max_stream_id_bidi has the wrong length", func() { + parameters[initialMaxStreamIDUniParameterID] = []byte{0x11, 0x22, 0x33, 0x44, 0x55} // should be 4 bytes + _, err := readTransportParamters(paramsMapToList(parameters)) + Expect(err).To(MatchError("wrong length for initial_max_stream_id_uni: 5 (expected 4)")) }) It("rejects the parameters if the initial_idle_timeout has the wrong length", func() { @@ -225,9 +232,10 @@ var _ = Describe("Transport Parameters", func() { Expect(values).To(HaveLen(5)) Expect(values).To(HaveKeyWithValue(initialMaxStreamDataParameterID, []byte{0xde, 0xad, 0xbe, 0xef})) Expect(values).To(HaveKeyWithValue(initialMaxDataParameterID, []byte{0xde, 0xca, 0xfb, 0xad})) - Expect(values).To(HaveKeyWithValue(initialMaxStreamIDParameterID, []byte{0xff, 0xff, 0xff, 0xff})) + Expect(values).To(HaveKeyWithValue(initialMaxStreamIDBiDiParameterID, []byte{0xff, 0xff, 0xff, 0xff})) Expect(values).To(HaveKeyWithValue(idleTimeoutParameterID, []byte{0xca, 0xfe})) Expect(values).To(HaveKeyWithValue(maxPacketSizeParameterID, []byte{0x5, 0xac})) // 1452 = 0x5ac + Expect(values).ToNot(HaveKey(initialMaxStreamIDUniParameterID)) }) It("request ommision of the connection ID", func() { diff --git a/internal/handshake/transport_parameters.go b/internal/handshake/transport_parameters.go index 918171bc1..e2d6df72e 100644 --- a/internal/handshake/transport_parameters.go +++ b/internal/handshake/transport_parameters.go @@ -113,9 +113,14 @@ func readTransportParamters(paramsList []transportParameter) (*TransportParamete return nil, fmt.Errorf("wrong length for initial_max_data: %d (expected 4)", len(p.Value)) } params.ConnectionFlowControlWindow = protocol.ByteCount(binary.BigEndian.Uint32(p.Value)) - case initialMaxStreamIDParameterID: + case initialMaxStreamIDBiDiParameterID: if len(p.Value) != 4 { - return nil, fmt.Errorf("wrong length for initial_max_stream_id: %d (expected 4)", len(p.Value)) + return nil, fmt.Errorf("wrong length for initial_max_stream_id_bidi: %d (expected 4)", len(p.Value)) + } + // TODO: handle this value + case initialMaxStreamIDUniParameterID: + if len(p.Value) != 4 { + return nil, fmt.Errorf("wrong length for initial_max_stream_id_uni: %d (expected 4)", len(p.Value)) } // TODO: handle this value case idleTimeoutParameterID: @@ -139,14 +144,15 @@ func readTransportParamters(paramsList []transportParameter) (*TransportParamete } // GetTransportParameters gets the parameters needed for the TLS handshake. +// It doesn't send the initial_max_stream_id_uni parameter, so the peer isn't allowed to open any unidirectional streams. func (p *TransportParameters) getTransportParameters() []transportParameter { initialMaxStreamData := make([]byte, 4) binary.BigEndian.PutUint32(initialMaxStreamData, uint32(p.StreamFlowControlWindow)) initialMaxData := make([]byte, 4) binary.BigEndian.PutUint32(initialMaxData, uint32(p.ConnectionFlowControlWindow)) - initialMaxStreamID := make([]byte, 4) + initialMaxStreamIDBiDi := make([]byte, 4) // TODO: use a reasonable value here - binary.BigEndian.PutUint32(initialMaxStreamID, math.MaxUint32) + binary.BigEndian.PutUint32(initialMaxStreamIDBiDi, math.MaxUint32) idleTimeout := make([]byte, 2) binary.BigEndian.PutUint16(idleTimeout, uint16(p.IdleTimeout/time.Second)) maxPacketSize := make([]byte, 2) @@ -154,7 +160,7 @@ func (p *TransportParameters) getTransportParameters() []transportParameter { params := []transportParameter{ {initialMaxStreamDataParameterID, initialMaxStreamData}, {initialMaxDataParameterID, initialMaxData}, - {initialMaxStreamIDParameterID, initialMaxStreamID}, + {initialMaxStreamIDBiDiParameterID, initialMaxStreamIDBiDi}, {idleTimeoutParameterID, idleTimeout}, {maxPacketSizeParameterID, maxPacketSize}, }