From c840bd4177047afbe1d1f9a7b748747f303f570d Mon Sep 17 00:00:00 2001
From: Marten Seemann <martenseemann@gmail.com>
Date: Wed, 15 Aug 2018 16:02:57 +0700
Subject: [PATCH] allow version negotiation between IETF QUIC and gQUIC, and
 vice versa

---
 client.go                                 | 34 ++++++++++-------------
 internal/handshake/crypto_setup_client.go | 26 +++++++++--------
 internal/protocol/version.go              | 11 ++++++++
 internal/protocol/version_test.go         |  9 ++++++
 4 files changed, 48 insertions(+), 32 deletions(-)

diff --git a/client.go b/client.go
index 83c7e4545..5c3f3c859 100644
--- a/client.go
+++ b/client.go
@@ -283,17 +283,14 @@ func (c *client) dial(ctx context.Context) error {
 }
 
 func (c *client) dialGQUIC(ctx context.Context) error {
-	var cerr error
-	for {
-		if err := c.createNewGQUICSession(); err != nil {
-			return err
-		}
-		cerr = c.establishSecureConnection(ctx)
-		if cerr != errCloseSessionForNewVersion {
-			break
-		}
+	if err := c.createNewGQUICSession(); err != nil {
+		return err
 	}
-	return cerr
+	err := c.establishSecureConnection(ctx)
+	if err == errCloseSessionForNewVersion {
+		return c.dial(ctx)
+	}
+	return err
 }
 
 func (c *client) dialTLS(ctx context.Context) error {
@@ -315,17 +312,14 @@ func (c *client) dialTLS(ctx context.Context) error {
 	mintConf.ServerName = c.hostname
 	c.mintConf = mintConf
 
-	var cerr error
-	for {
-		if err := c.createNewTLSSession(extHandler.GetPeerParams(), c.version); err != nil {
-			return err
-		}
-		cerr = c.establishSecureConnection(ctx)
-		if cerr != errCloseSessionForRetry && cerr != errCloseSessionForNewVersion {
-			break
-		}
+	if err := c.createNewTLSSession(extHandler.GetPeerParams(), c.version); err != nil {
+		return err
 	}
-	return cerr
+	err = c.establishSecureConnection(ctx)
+	if err == errCloseSessionForRetry || err == errCloseSessionForNewVersion {
+		return c.dial(ctx)
+	}
+	return err
 }
 
 // establishSecureConnection runs the session, and tries to establish a secure connection
diff --git a/internal/handshake/crypto_setup_client.go b/internal/handshake/crypto_setup_client.go
index 1324d5005..6687b8348 100644
--- a/internal/handshake/crypto_setup_client.go
+++ b/internal/handshake/crypto_setup_client.go
@@ -86,18 +86,20 @@ func NewCryptoSetupClient(
 	}
 	divNonceChan := make(chan struct{})
 	cs := &cryptoSetupClient{
-		cryptoStream:       cryptoStream,
-		hostname:           hostname,
-		connID:             connID,
-		version:            version,
-		certManager:        crypto.NewCertManager(tlsConfig),
-		params:             params,
-		keyDerivation:      crypto.DeriveQuicCryptoAESKeys,
-		nullAEAD:           nullAEAD,
-		paramsChan:         paramsChan,
-		handshakeEvent:     handshakeEvent,
-		initialVersion:     initialVersion,
-		negotiatedVersions: negotiatedVersions,
+		cryptoStream:   cryptoStream,
+		hostname:       hostname,
+		connID:         connID,
+		version:        version,
+		certManager:    crypto.NewCertManager(tlsConfig),
+		params:         params,
+		keyDerivation:  crypto.DeriveQuicCryptoAESKeys,
+		nullAEAD:       nullAEAD,
+		paramsChan:     paramsChan,
+		handshakeEvent: handshakeEvent,
+		initialVersion: initialVersion,
+		// The server might have sent greased versions in the Version Negotiation packet.
+		// We need strip those from the list, since they won't be included in the handshake tag.
+		negotiatedVersions: protocol.StripGreasedVersions(negotiatedVersions),
 		divNonceChan:       divNonceChan,
 		logger:             logger,
 	}
diff --git a/internal/protocol/version.go b/internal/protocol/version.go
index fa3c6723f..1ecfd35a1 100644
--- a/internal/protocol/version.go
+++ b/internal/protocol/version.go
@@ -152,3 +152,14 @@ func GetGreasedVersions(supported []VersionNumber) []VersionNumber {
 	copy(greased[randPos+1:], supported[randPos:])
 	return greased
 }
+
+// StripGreasedVersions strips all greased versions from a slice of versions
+func StripGreasedVersions(versions []VersionNumber) []VersionNumber {
+	realVersions := make([]VersionNumber, 0, len(versions))
+	for _, v := range versions {
+		if v&0x0f0f0f0f != 0x0a0a0a0a {
+			realVersions = append(realVersions, v)
+		}
+	}
+	return realVersions
+}
diff --git a/internal/protocol/version_test.go b/internal/protocol/version_test.go
index a2969cf09..01edc82b5 100644
--- a/internal/protocol/version_test.go
+++ b/internal/protocol/version_test.go
@@ -169,6 +169,15 @@ var _ = Describe("Version", func() {
 			Expect(isReservedVersion(greased[0])).To(BeTrue())
 		})
 
+		It("strips greased versions", func() {
+			v := SupportedVersions[0]
+			greased := GetGreasedVersions([]VersionNumber{v})
+			Expect(greased).To(HaveLen(2))
+			stripped := StripGreasedVersions(greased)
+			Expect(stripped).To(HaveLen(1))
+			Expect(stripped[0]).To(Equal(v))
+		})
+
 		It("creates greased lists of version numbers", func() {
 			supported := []VersionNumber{10, 18, 29}
 			for _, v := range supported {