Merge pull request #1178 from lucas-clemente/fix-1173

use an unbuffered chan for the client transport parameters

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,

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(&params))
+			Eventually(handler.GetPeerParams()).Should(Receive(&params))
 			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())
 			})
 		})
 	})

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,