From a136ceffeb741fff99105d90d191b8432cce478a Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Mon, 2 Oct 2017 13:51:38 +0700 Subject: [PATCH] implement the validation of the version negotiation for the client --- internal/handshake/crypto_setup_tls.go | 3 +- .../handshake/tls_extension_handler_client.go | 29 +++-- .../tls_extension_handler_client_test.go | 103 +++++++++++++++--- .../tls_extension_handler_server_test.go | 16 +-- session.go | 1 + 5 files changed, 119 insertions(+), 33 deletions(-) diff --git a/internal/handshake/crypto_setup_tls.go b/internal/handshake/crypto_setup_tls.go index 8f4acf8c..ceda3af9 100644 --- a/internal/handshake/crypto_setup_tls.go +++ b/internal/handshake/crypto_setup_tls.go @@ -66,6 +66,7 @@ func NewCryptoSetupTLSClient( transportParams *TransportParameters, aeadChanged chan<- protocol.EncryptionLevel, initialVersion protocol.VersionNumber, + supportedVersions []protocol.VersionNumber, version protocol.VersionNumber, ) (CryptoSetup, ParamsNegotiator, error) { mintConf, err := tlsToMintConfig(tlsConfig, protocol.PerspectiveClient) @@ -81,7 +82,7 @@ func NewCryptoSetupTLSClient( nullAEAD: crypto.NewNullAEAD(protocol.PerspectiveClient, version), keyDerivation: crypto.DeriveAESKeys, aeadChanged: aeadChanged, - extensionHandler: newExtensionHandlerClient(params, initialVersion, version), + extensionHandler: newExtensionHandlerClient(params, initialVersion, supportedVersions, version), }, params, nil } diff --git a/internal/handshake/tls_extension_handler_client.go b/internal/handshake/tls_extension_handler_client.go index 3a6a0752..8cf150ba 100644 --- a/internal/handshake/tls_extension_handler_client.go +++ b/internal/handshake/tls_extension_handler_client.go @@ -4,6 +4,8 @@ import ( "errors" "fmt" + "github.com/lucas-clemente/quic-go/qerr" + "github.com/bifurcation/mint" "github.com/bifurcation/mint/syntax" "github.com/lucas-clemente/quic-go/internal/protocol" @@ -12,17 +14,19 @@ import ( type extensionHandlerClient struct { params *paramsNegotiator - initialVersion protocol.VersionNumber - version protocol.VersionNumber + initialVersion protocol.VersionNumber + supportedVersions []protocol.VersionNumber + version protocol.VersionNumber } var _ mint.AppExtensionHandler = &extensionHandlerClient{} -func newExtensionHandlerClient(params *paramsNegotiator, initialVersion, version protocol.VersionNumber) *extensionHandlerClient { +func newExtensionHandlerClient(params *paramsNegotiator, initialVersion protocol.VersionNumber, supportedVersions []protocol.VersionNumber, version protocol.VersionNumber) *extensionHandlerClient { return &extensionHandlerClient{ - params: params, - initialVersion: initialVersion, - version: version, + params: params, + initialVersion: initialVersion, + supportedVersions: supportedVersions, + version: version, } } @@ -67,7 +71,18 @@ func (h *extensionHandlerClient) Receive(hType mint.HandshakeType, el *mint.Exte if _, err := syntax.Unmarshal(ext.data, eetp); err != nil { return err } - // TODO: check versions + serverSupportedVersions := make([]protocol.VersionNumber, len(eetp.SupportedVersions)) + for i, v := range eetp.SupportedVersions { + serverSupportedVersions[i] = protocol.VersionNumber(v) + } + // check that the current version is included in the supported versions + if !protocol.IsSupportedVersion(serverSupportedVersions, h.version) { + return qerr.Error(qerr.VersionNegotiationMismatch, "current version not included in the supported versions") + } + // if version negotiation was performed, check that we would have selected the current version based on the supported versions sent by the server + if h.version != h.initialVersion && h.version != protocol.ChooseSupportedVersion(h.supportedVersions, serverSupportedVersions) { + return qerr.Error(qerr.VersionNegotiationMismatch, "would have picked a different version") + } // check that the server sent the stateless reset token var foundStatelessResetToken bool diff --git a/internal/handshake/tls_extension_handler_client_test.go b/internal/handshake/tls_extension_handler_client_test.go index ffd3d8a7..d8561e5c 100644 --- a/internal/handshake/tls_extension_handler_client_test.go +++ b/internal/handshake/tls_extension_handler_client_test.go @@ -17,7 +17,7 @@ var _ = Describe("TLS Extension Handler, for the client", func() { BeforeEach(func() { pn := ¶msNegotiator{} - handler = newExtensionHandlerClient(pn, protocol.VersionWhatever, protocol.VersionWhatever) + handler = newExtensionHandlerClient(pn, protocol.VersionWhatever, nil, protocol.VersionWhatever) el = make(mint.ExtensionList, 0) }) @@ -53,14 +53,14 @@ var _ = Describe("TLS Extension Handler, for the client", func() { var fakeBody *tlsExtensionBody var parameters map[transportParameterID][]byte - paramaterMapToExtensionBody := func(paramMap map[transportParameterID][]byte) *tlsExtensionBody { - var params []transportParameter - for id, val := range paramMap { - params = append(params, transportParameter{id, val}) - } - body, err := syntax.Marshal(encryptedExtensionsTransportParameters{Parameters: params}) + addEncryptedExtensionsWithParameters := func(paramMap map[transportParameterID][]byte) { + body, err := syntax.Marshal(encryptedExtensionsTransportParameters{ + Parameters: parameterMapToList(paramMap), + SupportedVersions: []uint32{uint32(handler.version)}, + }) + Expect(err).ToNot(HaveOccurred()) + err = el.Add(&tlsExtensionBody{data: body}) Expect(err).ToNot(HaveOccurred()) - return &tlsExtensionBody{data: body} } BeforeEach(func() { @@ -75,9 +75,8 @@ var _ = Describe("TLS Extension Handler, for the client", func() { }) It("accepts the TransportParameters on the EncryptedExtensions message", func() { - err := el.Add(paramaterMapToExtensionBody(parameters)) - Expect(err).ToNot(HaveOccurred()) - err = handler.Receive(mint.HandshakeTypeEncryptedExtensions, &el) + addEncryptedExtensionsWithParameters(parameters) + err := handler.Receive(mint.HandshakeTypeEncryptedExtensions, &el) Expect(err).ToNot(HaveOccurred()) Expect(handler.params.GetSendStreamFlowControlWindow()).To(BeEquivalentTo(0x11223344)) }) @@ -116,18 +115,88 @@ var _ = Describe("TLS Extension Handler, for the client", func() { It("rejects TransportParameters if they don't contain the stateless reset token", func() { delete(parameters, statelessResetTokenParameterID) - err := el.Add(paramaterMapToExtensionBody(parameters)) - Expect(err).ToNot(HaveOccurred()) - err = handler.Receive(mint.HandshakeTypeEncryptedExtensions, &el) + addEncryptedExtensionsWithParameters(parameters) + err := handler.Receive(mint.HandshakeTypeEncryptedExtensions, &el) Expect(err).To(MatchError("server didn't sent stateless_reset_token")) }) It("errors if the stateless reset token has the wrong length", func() { parameters[statelessResetTokenParameterID] = bytes.Repeat([]byte{0}, 15) // should be 16 - err := el.Add(paramaterMapToExtensionBody(parameters)) - Expect(err).ToNot(HaveOccurred()) - err = handler.Receive(mint.HandshakeTypeEncryptedExtensions, &el) + addEncryptedExtensionsWithParameters(parameters) + err := handler.Receive(mint.HandshakeTypeEncryptedExtensions, &el) Expect(err).To(MatchError("wrong length for stateless_reset_token: 15 (expected 16)")) }) + + Context("Version Negotiation", func() { + It("accepts a valid version negotiation", func() { + handler.initialVersion = 13 + handler.version = 37 + handler.supportedVersions = []protocol.VersionNumber{13, 37, 42} + body, err := syntax.Marshal(encryptedExtensionsTransportParameters{ + Parameters: parameterMapToList(parameters), + SupportedVersions: []uint32{36, 37, 38}, + }) + Expect(err).ToNot(HaveOccurred()) + err = el.Add(&tlsExtensionBody{data: body}) + Expect(err).ToNot(HaveOccurred()) + err = handler.Receive(mint.HandshakeTypeEncryptedExtensions, &el) + Expect(err).ToNot(HaveOccurred()) + }) + + It("errors if the current version is not contained in the server's supported versions", func() { + handler.version = 42 + body, err := syntax.Marshal(encryptedExtensionsTransportParameters{ + SupportedVersions: []uint32{43, 44}, + }) + Expect(err).ToNot(HaveOccurred()) + err = el.Add(&tlsExtensionBody{data: body}) + Expect(err).ToNot(HaveOccurred()) + err = handler.Receive(mint.HandshakeTypeEncryptedExtensions, &el) + Expect(err).To(MatchError("VersionNegotiationMismatch: current version not included in the supported versions")) + }) + + It("errors if version negotiation was performed, but would have picked a different version based on the supported version list", func() { + handler.version = 42 + handler.initialVersion = 41 + handler.supportedVersions = []protocol.VersionNumber{43, 42, 41} + serverSupportedVersions := []protocol.VersionNumber{42, 43} + // check that version negotiation would have led us to pick version 43 + Expect(protocol.ChooseSupportedVersion(handler.supportedVersions, serverSupportedVersions)).To(Equal(protocol.VersionNumber(43))) + ssv := make([]uint32, len(serverSupportedVersions)) + for i, v := range serverSupportedVersions { + ssv[i] = uint32(v) + } + body, err := syntax.Marshal(encryptedExtensionsTransportParameters{ + SupportedVersions: ssv, + }) + Expect(err).ToNot(HaveOccurred()) + err = el.Add(&tlsExtensionBody{data: body}) + Expect(err).ToNot(HaveOccurred()) + err = handler.Receive(mint.HandshakeTypeEncryptedExtensions, &el) + Expect(err).To(MatchError("VersionNegotiationMismatch: would have picked a different version")) + }) + + It("doesn't error if it would have picked a different version based on the supported version list, if no version negotiation was performed", func() { + handler.version = 42 + handler.initialVersion = 42 // version == initialVersion means no version negotiation was performed + handler.supportedVersions = []protocol.VersionNumber{43, 42, 41} + serverSupportedVersions := []protocol.VersionNumber{42, 43} + // check that version negotiation would have led us to pick version 43 + Expect(protocol.ChooseSupportedVersion(handler.supportedVersions, serverSupportedVersions)).To(Equal(protocol.VersionNumber(43))) + ssv := make([]uint32, len(serverSupportedVersions)) + for i, v := range serverSupportedVersions { + ssv[i] = uint32(v) + } + body, err := syntax.Marshal(encryptedExtensionsTransportParameters{ + Parameters: parameterMapToList(parameters), + SupportedVersions: ssv, + }) + Expect(err).ToNot(HaveOccurred()) + err = el.Add(&tlsExtensionBody{data: body}) + Expect(err).ToNot(HaveOccurred()) + err = handler.Receive(mint.HandshakeTypeEncryptedExtensions, &el) + Expect(err).ToNot(HaveOccurred()) + }) + }) }) }) diff --git a/internal/handshake/tls_extension_handler_server_test.go b/internal/handshake/tls_extension_handler_server_test.go index c5857ded..5f902289 100644 --- a/internal/handshake/tls_extension_handler_server_test.go +++ b/internal/handshake/tls_extension_handler_server_test.go @@ -10,6 +10,14 @@ import ( . "github.com/onsi/gomega" ) +func parameterMapToList(paramMap map[transportParameterID][]byte) []transportParameter { + var params []transportParameter + for id, val := range paramMap { + params = append(params, transportParameter{id, val}) + } + return params +} + var _ = Describe("TLS Extension Handler, for the server", func() { var handler *extensionHandlerServer var el mint.ExtensionList @@ -50,14 +58,6 @@ var _ = Describe("TLS Extension Handler, for the server", func() { var fakeBody *tlsExtensionBody var parameters map[transportParameterID][]byte - parameterMapToList := func(paramMap map[transportParameterID][]byte) []transportParameter { - var params []transportParameter - for id, val := range paramMap { - params = append(params, transportParameter{id, val}) - } - return params - } - addClientHelloWithParameters := func(paramMap map[transportParameterID][]byte) { body, err := syntax.Marshal(clientHelloTransportParameters{Parameters: parameterMapToList(paramMap)}) Expect(err).ToNot(HaveOccurred()) diff --git a/session.go b/session.go index c5b9e4e9..5417103c 100644 --- a/session.go +++ b/session.go @@ -223,6 +223,7 @@ func (s *session) setup( transportParams, aeadChanged, initialVersion, + s.config.Versions, s.version, ) } else {