From c9b95abe7ee61e1ec68400f169c44ada4900d691 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 8 Feb 2018 11:04:27 +0800 Subject: [PATCH] use an unbuffered chan for the client transport parameters The client reads the transport parameters from the Encrypted Extensions message. These transport parameters are passed to the session's run loop's select statement via a channel. We have to use an unbuffered channel here to make sure that the session actually processes the transport parameters immediately. --- .../handshake/tls_extension_handler_client.go | 5 ++- .../tls_extension_handler_client_test.go | 44 +++++++++++++++++-- .../handshake/tls_extension_handler_server.go | 2 + 3 files changed, 46 insertions(+), 5 deletions(-) diff --git a/internal/handshake/tls_extension_handler_client.go b/internal/handshake/tls_extension_handler_client.go index 20d2d06ba..37eb6a958 100644 --- a/internal/handshake/tls_extension_handler_client.go +++ b/internal/handshake/tls_extension_handler_client.go @@ -31,7 +31,10 @@ func NewExtensionHandlerClient( supportedVersions []protocol.VersionNumber, version protocol.VersionNumber, ) TLSExtensionHandler { - paramsChan := make(chan TransportParameters, 1) + // The client reads the transport parameters from the Encrypted Extensions message. + // The paramsChan is used in the session's run loop's select statement. + // We have to use an unbuffered channel here to make sure that the session actually processes the transport parameters immediately. + paramsChan := make(chan TransportParameters) return &extensionHandlerClient{ ourParams: params, paramsChan: paramsChan, diff --git a/internal/handshake/tls_extension_handler_client_test.go b/internal/handshake/tls_extension_handler_client_test.go index 05cfae5ea..d0a0e3296 100644 --- a/internal/handshake/tls_extension_handler_client_test.go +++ b/internal/handshake/tls_extension_handler_client_test.go @@ -74,13 +74,33 @@ var _ = Describe("TLS Extension Handler, for the client", func() { } }) + It("blocks until the transport parameters are read", func() { + done := make(chan struct{}) + go func() { + defer GinkgoRecover() + addEncryptedExtensionsWithParameters(parameters) + err := handler.Receive(mint.HandshakeTypeEncryptedExtensions, &el) + Expect(err).ToNot(HaveOccurred()) + close(done) + }() + Consistently(done).ShouldNot(BeClosed()) + Expect(handler.GetPeerParams()).To(Receive()) + Eventually(done).Should(BeClosed()) + }) + It("accepts the TransportParameters on the EncryptedExtensions message", func() { - addEncryptedExtensionsWithParameters(parameters) - err := handler.Receive(mint.HandshakeTypeEncryptedExtensions, &el) - Expect(err).ToNot(HaveOccurred()) + done := make(chan struct{}) + go func() { + defer GinkgoRecover() + addEncryptedExtensionsWithParameters(parameters) + err := handler.Receive(mint.HandshakeTypeEncryptedExtensions, &el) + Expect(err).ToNot(HaveOccurred()) + close(done) + }() var params TransportParameters - Expect(handler.GetPeerParams()).To(Receive(¶ms)) + Eventually(handler.GetPeerParams()).Should(Receive(¶ms)) Expect(params.StreamFlowControlWindow).To(BeEquivalentTo(0x11223344)) + Eventually(done).Should(BeClosed()) }) It("errors if the EncryptedExtensions message doesn't contain TransportParameters", func() { @@ -131,6 +151,13 @@ var _ = Describe("TLS Extension Handler, for the client", func() { Context("Version Negotiation", func() { It("accepts a valid version negotiation", func() { + done := make(chan struct{}) + go func() { + defer GinkgoRecover() + Eventually(handler.GetPeerParams()).Should(Receive()) + close(done) + }() + handler.initialVersion = 13 handler.version = 37 handler.supportedVersions = []protocol.VersionNumber{13, 37, 42} @@ -144,6 +171,7 @@ var _ = Describe("TLS Extension Handler, for the client", func() { Expect(err).ToNot(HaveOccurred()) err = handler.Receive(mint.HandshakeTypeEncryptedExtensions, &el) Expect(err).ToNot(HaveOccurred()) + Eventually(done).Should(BeClosed()) }) It("errors if the current version doesn't match negotiated_version", func() { @@ -200,6 +228,13 @@ var _ = Describe("TLS Extension Handler, for the client", func() { }) 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() { + done := make(chan struct{}) + go func() { + defer GinkgoRecover() + Eventually(handler.GetPeerParams()).Should(Receive()) + close(done) + }() + handler.version = 42 handler.initialVersion = 42 // version == initialVersion means no version negotiation was performed handler.supportedVersions = []protocol.VersionNumber{43, 42, 41} @@ -222,6 +257,7 @@ var _ = Describe("TLS Extension Handler, for the client", func() { Expect(err).ToNot(HaveOccurred()) err = handler.Receive(mint.HandshakeTypeEncryptedExtensions, &el) Expect(err).ToNot(HaveOccurred()) + Eventually(done).Should(BeClosed()) }) }) }) diff --git a/internal/handshake/tls_extension_handler_server.go b/internal/handshake/tls_extension_handler_server.go index 3e7e2705f..1ad718514 100644 --- a/internal/handshake/tls_extension_handler_server.go +++ b/internal/handshake/tls_extension_handler_server.go @@ -29,6 +29,8 @@ func NewExtensionHandlerServer( supportedVersions []protocol.VersionNumber, version protocol.VersionNumber, ) TLSExtensionHandler { + // Processing the ClientHello is performed statelessly (and from a single go-routine). + // Therefore, we have to use a buffered chan to pass the transport parameters to that go routine. paramsChan := make(chan TransportParameters, 1) return &extensionHandlerServer{ ourParams: params,